Re: [3.12-rc1] Dependency on module-init-tools = 3.11 ?

2013-09-13 Thread Tetsuo Handa 
Waiman Long wrote:
 I would like to report that I also have the same boot problem on a 
 RHEL6.4 box with the crypto patch. My workaround is to force kernel 
 build to have the crc_t10dif code built-in by changing the config file:
 
 4889c4889
  CONFIG_CRYPTO_CRCT10DIF=m
 ---
   CONFIG_CRYPTO_CRCT10DIF=y
 5002c5002
  CONFIG_CRC_T10DIF=m
 ---
   CONFIG_CRC_T10DIF=y
 
 This solved the boot problem without any additional patch.  Do you think 
 you should consider changing the configuration default to y instead of 
 m or doesn't allow the m option at all?

That was proposed but not accepted.
https://lkml.org/lkml/2013/7/17/543

You should choose CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y in your kernel config
if your CPU supports PCLMULQDQ.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [3.12-rc1] Dependency on module-init-tools = 3.11 ?

2013-09-12 Thread Tetsuo Handa 
Herbert Xu wrote:
 The trouble is not all distros will include the softdep modules in
 the initramfs.  So for now I think we will have to live with a fallback.

I see.

Herbert Xu wrote:
 OK, can you please try this patch on top of the current tree?
 
 This way at least you'll have a working system until your initramfs
 tool is fixed to do the right thing.

I tested the patch and confirmed that the boot failure was solved.

But arch/x86/crypto/crct10dif-pclmul.ko is not included into initramfs and
therefore we cannot benefit from PCLMULQDQ version.

-- before applying patch --
kernel/arch/x86/crypto/crct10dif-pclmul.ko: kernel/crypto/crct10dif.ko
kernel/crypto/crct10dif.ko:
kernel/drivers/scsi/lpfc/lpfc.ko: kernel/drivers/scsi/scsi_transport_fc.ko 
kernel/drivers/scsi/scsi_tgt.ko kernel/lib/crc-t10dif.ko
kernel/drivers/scsi/sd_mod.ko: kernel/lib/crc-t10dif.ko
kernel/drivers/scsi/scsi_debug.ko: kernel/lib/crc-t10dif.ko
kernel/lib/crc-t10dif.ko:
-- before applying patch --

-- after applying patch --
kernel/arch/x86/crypto/crct10dif-pclmul.ko: kernel/crypto/crct10dif_common.ko
kernel/crypto/crct10dif_common.ko:
kernel/crypto/crct10dif_generic.ko: kernel/crypto/crct10dif_common.ko
kernel/drivers/scsi/lpfc/lpfc.ko: kernel/drivers/scsi/scsi_transport_fc.ko 
kernel/drivers/scsi/scsi_tgt.ko kernel/lib/crc-t10dif.ko 
kernel/crypto/crct10dif_common.ko
kernel/drivers/scsi/sd_mod.ko: kernel/lib/crc-t10dif.ko 
kernel/crypto/crct10dif_common.ko
kernel/drivers/scsi/scsi_debug.ko: kernel/lib/crc-t10dif.ko 
kernel/crypto/crct10dif_common.ko
kernel/lib/crc-t10dif.ko: kernel/crypto/crct10dif_common.ko
-- after applying patch --
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to moduledependency.

2013-07-19 Thread Tetsuo Handa 
Tim Chen wrote:
 On Fri, 2013-07-19 at 16:37 -0700, Tim Chen wrote:
  Herbert,
  
  I've tried the module alias approach (see my earlier mail with patch) 
  but it didn't seem to load things properly.  Can you check to see if 
  there's anything I did incorrectly.
  
  Tim
 
 I fixed a missing request_module statement in crct10dif library.  
 So now things work if I have the following config:
 
 CONFIG_CRYPTO_CRCT10DIF=m
 CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
 CONFIG_CRC_T10DIF=m
 
 However, when I have the library and generic algorithm compiled in,
 I do not see the PCLMULQDQ version loaded.
 
 CONFIG_CRYPTO_CRCT10DIF=y
 CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
 CONFIG_CRC_T10DIF=y
 
 Perhaps I am initiating the crct10dif library at a really early
 stage when things are compiled in, where the module is not in 
 initramfs?  In that case, perhaps we should only allow 
 PCLMUL version to be compiled in
 and not exist as a module?

I think that use of request_module(crct10dif) does not help loading
crct10dif-pclmul.ko when CONFIG_CRC_T10DIF=y CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m ,
for there is no / directory (note that the initramfs is not yet mounted as /
for loading modules which are not in vmlinux) when any module_init() functions
which are in vmlinux are called.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to moduledependency.

2013-07-19 Thread Tetsuo Handa 
Herbert Xu wrote:
 On Fri, Jul 19, 2013 at 06:31:04PM -0700, Tim Chen wrote:
 
  However, when I have the library and generic algorithm compiled in,
  I do not see the PCLMULQDQ version loaded.
  
  CONFIG_CRYPTO_CRCT10DIF=y
  CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
  CONFIG_CRC_T10DIF=y
 
 That is completely expected.  I don't really think we need to
 do anything about this case.  After all, if the admin wants to
 use the optimised version for CRC_T10DIF then they could simply
 compile that in as well.
 

Wow! ;-)

But I'd expect something like

 static int __init crc_t10dif_mod_init(void)
 {
+#if !defined(CONFIG_CRC_T10DIF_MODULE)  
defined(CONFIG_CRYPTO_CRCT10DIF_PCLMUL_MODULE)
+   printk(KERN_WARNING Consider CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y for 
better performance\n);
+#endif
crct10dif_tfm = crypto_alloc_shash(crct10dif, 0, 0);
return PTR_RET(crct10dif_tfm);
 }

because the admin might not be aware of this implication.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3.11-rc1] crypto: Fix boot failure due tomoduledependency.

2013-07-17 Thread Tetsuo Handa 
Tim Chen wrote:
 Herbert, seems like modules.dep generator wants explicit
 
 - select CRYPTO_CRCT10DIF
 + depends on CRYPTO_CRCT10DIF
 
 But it seems to me like it should have known CRC_T10DIF needs
 CRYPTO_CRCT10DIF when we do 
   select CRYPTO_CRCT10DIF
 
 Your thoughts?

select cannot tell /sbin/depmod that there is dependency, for /sbin/depmod
calculates dependency by enumerating symbols in each module rather than by
parsing Kconfig files which depends on kernel-source_files_installed = y.

Therefore, I think possible solutions are either

  (a) built-in the dependent modules

  # grep crc /lib/modules/3.11.0-rc1+/modules.dep
  kernel/drivers/scsi/sd_mod.ko: kernel/lib/crc-t10dif.ko
  kernel/lib/crc-t10dif.ko:

or

  (b) embed explicit reference to the dependent module's symbols

  # grep crc /lib/modules/3.11.0-rc1+/modules.dep
  kernel/arch/x86/crypto/crct10dif-pclmul.ko: kernel/crypto/crct10dif.ko
  kernel/crypto/crct10dif.ko:
  kernel/drivers/scsi/sd_mod.ko: kernel/lib/crc-t10dif.ko 
kernel/arch/x86/crypto/crct10dif-pclmul.ko kernel/crypto/crct10dif.ko
  kernel/lib/crc-t10dif.ko: kernel/arch/x86/crypto/crct10dif-pclmul.ko 
kernel/crypto/crct10dif.ko

.

Two patches ((a) and (b) respectively) follow, but I think patch (b) will not
work unless additional change

  static int __init crct10dif_intel_mod_init(void)
  {
  if (x86_match_cpu(crct10dif_cpu_id))
  return crypto_register_shash(alg);
  return 0;
  }

  static void __exit crct10dif_intel_mod_fini(void)
  {
  if (x86_match_cpu(crct10dif_cpu_id))
  crypto_unregister_shash(alg);
  }

is made, for currently crct10dif-pclmul.ko cannot be loaded on
!X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ) systems.



From d8d9b7c3e5be9c5a6198dac6fe7279ca904343a8 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa penguin-ker...@i-love.sakura.ne.jp
Date: Wed, 17 Jul 2013 19:45:19 +0900
Subject: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

Commit 2d31e518 crypto: crct10dif - Wrap crc_t10dif function all to use crypto
transform framework was added without telling that crc-t10dif.ko depends on
crct10dif.ko. This resulted in boot failure because sd_mod.ko depends on
crc-t10dif.ko but crct10dif.ko is not loaded automatically.

Fix this by changing crct10dif.ko and crct10dif-pclmul.ko from tristate to
bool so that suitable version is chosen.

Signed-off-by: Tetsuo Handa penguin-ker...@i-love.sakura.ne.jp
---
 crypto/Kconfig |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 69ce573..dd3b79e 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -377,7 +377,7 @@ config CRYPTO_CRC32_PCLMUL
  and gain better performance as compared with the table implementation.
 
 config CRYPTO_CRCT10DIF
-   tristate CRCT10DIF algorithm
+   bool CRCT10DIF algorithm
select CRYPTO_HASH
help
  CRC T10 Data Integrity Field computation is being cast as
@@ -385,7 +385,7 @@ config CRYPTO_CRCT10DIF
  transforms to be used if they are available.
 
 config CRYPTO_CRCT10DIF_PCLMUL
-   tristate CRCT10DIF PCLMULQDQ hardware acceleration
+   bool CRCT10DIF PCLMULQDQ hardware acceleration
depends on X86  64BIT  CRC_T10DIF
select CRYPTO_HASH
help
-- 
1.7.1



From 153e209fc9a7e1df42555829c396ee9ed53e83d0 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa penguin-ker...@i-love.sakura.ne.jp
Date: Wed, 17 Jul 2013 20:23:15 +0900
Subject: [PATCH] [PATCH 3.11-rc1] crypto: Fix boot failure due to module 
dependency.

Commit 2d31e518 crypto: crct10dif - Wrap crc_t10dif function all to use crypto
transform framework was added without telling that crc-t10dif.ko depends on
crct10dif.ko. This resulted in boot failure because sd_mod.ko depends on
crc-t10dif.ko but crct10dif.ko is not loaded automatically.

Fix this by describing crc-t10dif.ko depends on crct10dif.ko.

Signed-off-by: Tetsuo Handa penguin-ker...@i-love.sakura.ne.jp
---
 arch/x86/crypto/crct10dif-pclmul_glue.c |6 ++
 lib/crc-t10dif.c|7 +++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/x86/crypto/crct10dif-pclmul_glue.c 
b/arch/x86/crypto/crct10dif-pclmul_glue.c
index 7845d7f..2964608 100644
--- a/arch/x86/crypto/crct10dif-pclmul_glue.c
+++ b/arch/x86/crypto/crct10dif-pclmul_glue.c
@@ -149,3 +149,9 @@ MODULE_LICENSE(GPL);
 
 MODULE_ALIAS(crct10dif);
 MODULE_ALIAS(crct10dif-pclmul);
+
+/* Dummy for describing module dependency. */
+#if defined(CONFIG_CRYPTO_CRCT10DIF_PCLMUL_MODULE)
+const char crct10dif_pclmul;
+EXPORT_SYMBOL(crct10dif_pclmul);
+#endif
diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c
index fe3428c..376f795 100644
--- a/lib/crc-t10dif.c
+++ b/lib/crc-t10dif.c
@@ -38,6 +38,13 @@ EXPORT_SYMBOL(crc_t10dif);
 
 static

Re: [PATCH 3.11-rc1] crypto: Fix boot failure duetomoduledependency.

2013-07-17 Thread Tetsuo Handa 
Tim Chen wrote:
  Therefore, I think possible solutions are either
  
(a) built-in the dependent modules
  
# grep crc /lib/modules/3.11.0-rc1+/modules.dep
kernel/drivers/scsi/sd_mod.ko: kernel/lib/crc-t10dif.ko
kernel/lib/crc-t10dif.ko:
 
 This approach will increase kernel size for those who are not using
 t10dif so some people may object.  
 BTW, The PCLMULQDQ version does not need to be build in.

sd_mod.ko must choose one from versions available as of loading sd_mod.ko.
Although it is not needed to built-in the PCLMULQDQ version for fixing the boot
failure, it is needed to built-in the PCLMULQDQ version for getting performance
improvement when PCLMULQDQ is supported.

 Your approach is quite complicated.  I think something simpler like the
 following will work:

We cannot benefit from PCLMULQDQ. Is it acceptable for you?
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-17 Thread Tetsuo Handa 
Tim Chen wrote:
   Your approach is quite complicated.  I think something simpler like the
   following will work:
 
  We cannot benefit from PCLMULQDQ. Is it acceptable for you?
 
 
 The following code in crct10dif-pclmul_glue.c
 
 static const struct x86_cpu_id crct10dif_cpu_id[] = {
 X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
 {}
 };
 MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
 
 will put the module in the device table and get the module
 loaded, as long as the cpu support PCLMULQDQ. So we should be able
 to benefit.

Excuse me, how can crct10dif-pclmul.ko get loaded automatically?
Did you test CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m with below debug message?

diff --git a/arch/x86/crypto/crct10dif-pclmul_glue.c 
b/arch/x86/crypto/crct10dif-pclmul_glue.c
index 7845d7f..a8a95aa 100644
--- a/arch/x86/crypto/crct10dif-pclmul_glue.c
+++ b/arch/x86/crypto/crct10dif-pclmul_glue.c
@@ -129,9 +129,10 @@ MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
 
 static int __init crct10dif_intel_mod_init(void)
 {
+   printk(KERN_WARNING ** Checking for X86_FEATURE_PCLMULQDQ\n);
if (!x86_match_cpu(crct10dif_cpu_id))
return -ENODEV;
-
+   printk(KERN_WARNING ** Registering crct10dif-pclmul\n);
return crypto_register_shash(alg);
 }

As far as I tested, crct10dif-pclmul.ko will not be loaded unless manually
adding modprobe crct10dif-pclmul to initramfs's /init or choosing
CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.

 So as long as the crct10dif.ko and crct10dif-pclmul.ko are loaded,
 the pclmulqdq t10dif will have a higher priority and get allocated
 and used.

What I'm talking are

  (1) Since mkinitramfs is unable to know that crct10dif-pclmul.ko has higher
  priority than crct10dif.ko , mkinitramfs will not include
  modprobe crct10dif-pclmul line in the generated initramfs.

  (2) In order to get benefit from PCLMULQDQ, users have to manually make sure
  that modprobe crct10dif-pclmul is called before crc-t10dif.ko (which is
  loaded before sd_mod.ko is loaded) is loaded by their initramfs's /init
  script.

  (3) The cause of (1) is that crct10dif-pclmul.ko will not be loaded
  automatically unless choosing CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.

  (4) The cause of (3) is that modules.dep does not describe that users will
  benefit by loading crct10dif-pclmul.ko before loading crc-t10dif.ko .

  (5) Currently crct10dif-pclmul.ko cannot be loaded if PCLMULQDQ is not
  supported. This leads to boot failure (since sd_mod.ko cannot be loaded)
  if modules.dep says that crct10dif-pclmul.ko is required by
  crc-t10dif.ko.

  (6) To solve (4) and (5), modules.dep should say crct10dif-pclmul.ko is
  preferred for crc-t10dif.ko but is not required by crc-t10dif.ko.
  But there is no such mechanism. Thus, currently available choice is
  allow loading crct10dif-pclmul.ko even if PCLMULQDQ is not supported
  or ignore errors by built-in the crct10dif-pclmul.ko module.

My patch (b) seems to be complicated but is required in order to solve (4)
without asking users to manually add modprobe crct10dif-pclmul into their
initramfs. If we choose patch (b) rather than patch (a), we need to solve (5).
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html