[PATCH 1/3] kernel/srcu: merge common code into a macro

[PATCH 1/3] kernel/srcu: merge common code into a macro

[Sebastian Andrzej Siewior]

DEFINE_SRCU() and DEFINE_STATIC_SRCU() does the same thing except for
the "static" attribute. This patch moves the common pieces into
_DEFINE_SRCU() which is used by the the former macros either adding the
static attribute or not.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/srcu.h |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 6eb691b..d04acb8 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -102,13 +102,13 @@ void process_srcu(struct work_struct *work);
  * define and init a srcu struct at build time.
  * dont't call init_srcu_struct() nor cleanup_srcu_struct() on it.
  */
-#define DEFINE_SRCU(name)						\
+#define _DEFINE_SRCU(name, mod)						\
 	static DEFINE_PER_CPU(struct srcu_struct_array, name##_srcu_array);\
-	struct srcu_struct name = __SRCU_STRUCT_INIT(name);
+	mod struct srcu_struct name =					\
+				__SRCU_STRUCT_INIT(name);
 
-#define DEFINE_STATIC_SRCU(name)					\
-	static DEFINE_PER_CPU(struct srcu_struct_array, name##_srcu_array);\
-	static struct srcu_struct name = __SRCU_STRUCT_INIT(name);
+#define DEFINE_SRCU(name)		_DEFINE_SRCU(name, )
+#define DEFINE_STATIC_SRCU(name)	_DEFINE_SRCU(name, static)
 
 /**
  * call_srcu() - Queue a callback for invocation after an SRCU grace period
-- 
1.7.10.4

[PATCH 2/3] kernel/SRCU: provide a static initializer

[Sebastian Andrzej Siewior]

There are macros for static initializer for the three out of four
possible notifier types, that are:
	ATOMIC_NOTIFIER_HEAD()
	BLOCKING_NOTIFIER_HEAD()
	RAW_NOTIFIER_HEAD()

This patch provides a static initilizer for the forth type to make it
complete.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/notifier.h |   26 +++++++++++++++++++++-----
 include/linux/srcu.h     |    6 +++---
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index d65746e..6bfd703 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -42,9 +42,7 @@
  * in srcu_notifier_call_chain(): no cache bounces and no memory barriers.
  * As compensation, srcu_notifier_chain_unregister() is rather expensive.
  * SRCU notifier chains should be used when the chain will be called very
- * often but notifier_blocks will seldom be removed.  Also, SRCU notifier
- * chains are slightly more difficult to use because they require special
- * runtime initialization.
+ * often but notifier_blocks will seldom be removed.
  */
 
 struct notifier_block {
@@ -85,7 +83,7 @@ struct srcu_notifier_head {
 		(name)->head = NULL;		\
 	} while (0)
 
-/* srcu_notifier_heads must be initialized and cleaned up dynamically */
+/* srcu_notifier_heads must be cleaned up dynamically */
 extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
 #define srcu_cleanup_notifier_head(name)	\
 		cleanup_srcu_struct(&(name)->srcu);
@@ -98,7 +96,13 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
 		.head = NULL }
 #define RAW_NOTIFIER_INIT(name)	{				\
 		.head = NULL }
-/* srcu_notifier_heads cannot be initialized statically */
+
+#define SRCU_NOTIFIER_INIT(name, pcpu)				\
+	{							\
+		.mutex = __MUTEX_INITIALIZER(name.mutex),	\
+		.head = NULL,					\
+		.srcu = __SRCU_STRUCT_INIT(name.srcu, pcpu),	\
+	}
 
 #define ATOMIC_NOTIFIER_HEAD(name)				\
 	struct atomic_notifier_head name =			\
@@ -110,6 +114,18 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
 	struct raw_notifier_head name =				\
 		RAW_NOTIFIER_INIT(name)
 
+#define _SRCU_NOTIFIER_HEAD(name, mod)				\
+	static DEFINE_PER_CPU(struct srcu_struct_array,		\
+			name##_head_srcu_array);		\
+	mod struct srcu_notifier_head name =			\
+			SRCU_NOTIFIER_INIT(name, name##_head_srcu_array)
+
+#define SRCU_NOTIFIER_HEAD(name)				\
+	_SRCU_NOTIFIER_HEAD(name, )
+
+#define SRCU_NOTIFIER_HEAD_STATIC(name)				\
+	_SRCU_NOTIFIER_HEAD(name, static)
+
 #ifdef __KERNEL__
 
 extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index d04acb8..fe9efd4 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -84,10 +84,10 @@ int init_srcu_struct(struct srcu_struct *sp);
 
 void process_srcu(struct work_struct *work);
 
-#define __SRCU_STRUCT_INIT(name)					\
+#define __SRCU_STRUCT_INIT(name, pcpu_name)				\
 	{								\
 		.completed = -300,					\
-		.per_cpu_ref = &name##_srcu_array,			\
+		.per_cpu_ref = &pcpu_name,				\
 		.queue_lock = __SPIN_LOCK_UNLOCKED(name.queue_lock),	\
 		.running = false,					\
 		.batch_queue = RCU_BATCH_INIT(name.batch_queue),	\
@@ -105,7 +105,7 @@ void process_srcu(struct work_struct *work);
 #define _DEFINE_SRCU(name, mod)						\
 	static DEFINE_PER_CPU(struct srcu_struct_array, name##_srcu_array);\
 	mod struct srcu_struct name =					\
-				__SRCU_STRUCT_INIT(name);
+				__SRCU_STRUCT_INIT(name, name##_srcu_array);
 
 #define DEFINE_SRCU(name)		_DEFINE_SRCU(name, )
 #define DEFINE_STATIC_SRCU(name)	_DEFINE_SRCU(name, static)
-- 
1.7.10.4

[PATCH 3/3] cpufreq: use static initializer for the SRCU notifier

[Sebastian Andrzej Siewior]

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/cpufreq/cpufreq.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1f93dbd..cd5acda 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -117,12 +117,11 @@ static void handle_update(struct work_struct *work);
  * The mutex locks both lists.
  */
 static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list);
-static struct srcu_notifier_head cpufreq_transition_notifier_list;
+SRCU_NOTIFIER_HEAD_STATIC(cpufreq_transition_notifier_list);
 
 static bool init_cpufreq_transition_notifier_list_called;
 static int __init init_cpufreq_transition_notifier_list(void)
 {
-	srcu_init_notifier_head(&cpufreq_transition_notifier_list);
 	init_cpufreq_transition_notifier_list_called = true;
 	return 0;
 }
-- 
1.7.10.4

Re: [PATCH 1/3] kernel/srcu: merge common code into a macro

[Joe Perches]

On Tue, 2013-03-19 at 15:16 +0100, Sebastian Andrzej Siewior wrote:
> DEFINE_SRCU() and DEFINE_STATIC_SRCU() does the same thing except for
> the "static" attribute. This patch moves the common pieces into
> _DEFINE_SRCU() which is used by the the former macros either adding the
> static attribute or not.
[]
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
[]
> +#define DEFINE_SRCU(name)		_DEFINE_SRCU(name, )
> +#define DEFINE_STATIC_SRCU(name)	_DEFINE_SRCU(name, static)

I think the use of an empty argument, even in
a macro, unsightly.

Re: [PATCH 1/3] kernel/srcu: merge common code into a macro

[Sebastian Andrzej Siewior]

On 03/19/2013 05:22 PM, Joe Perches wrote:
>> +#define DEFINE_SRCU(name)		_DEFINE_SRCU(name, )
>> +#define DEFINE_STATIC_SRCU(name)	_DEFINE_SRCU(name, static)
> 
> I think the use of an empty argument, even in
> a macro, unsightly.

__wait_event_lock_irq() => __wait_event_lock_irq() is a prominent user.
I would prefer to avoid adding something like /* nothing */.

Sebastian

Re: [PATCH 1/3] kernel/srcu: merge common code into a macro

[Lai Jiangshan]

On 03/19/2013 10:16 PM, Sebastian Andrzej Siewior wrote:
> DEFINE_SRCU() and DEFINE_STATIC_SRCU() does the same thing except for
> the "static" attribute. This patch moves the common pieces into
> _DEFINE_SRCU() which is used by the the former macros either adding the
> static attribute or not.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  include/linux/srcu.h |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Hi, Sebastian

The patch hurts readability.
The original code are simple enough, merging them as one macro
gives us no benefit.

Thanks
Lai.


> 
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index 6eb691b..d04acb8 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -102,13 +102,13 @@ void process_srcu(struct work_struct *work);
>   * define and init a srcu struct at build time.
>   * dont't call init_srcu_struct() nor cleanup_srcu_struct() on it.
>   */
> -#define DEFINE_SRCU(name)						\
> +#define _DEFINE_SRCU(name, mod)						\
>  	static DEFINE_PER_CPU(struct srcu_struct_array, name##_srcu_array);\
> -	struct srcu_struct name = __SRCU_STRUCT_INIT(name);
> +	mod struct srcu_struct name =					\
> +				__SRCU_STRUCT_INIT(name);
>  
> -#define DEFINE_STATIC_SRCU(name)					\
> -	static DEFINE_PER_CPU(struct srcu_struct_array, name##_srcu_array);\
> -	static struct srcu_struct name = __SRCU_STRUCT_INIT(name);
> +#define DEFINE_SRCU(name)		_DEFINE_SRCU(name, )
> +#define DEFINE_STATIC_SRCU(name)	_DEFINE_SRCU(name, static)
>  
>  /**
>   * call_srcu() - Queue a callback for invocation after an SRCU grace period

Re: [PATCH 2/3] kernel/SRCU: provide a static initializer

[Lai Jiangshan]

On 03/19/2013 10:16 PM, Sebastian Andrzej Siewior wrote:
> There are macros for static initializer for the three out of four
> possible notifier types, that are:
> 	ATOMIC_NOTIFIER_HEAD()
> 	BLOCKING_NOTIFIER_HEAD()
> 	RAW_NOTIFIER_HEAD()
> 
> This patch provides a static initilizer for the forth type to make it
> complete.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  include/linux/notifier.h |   26 +++++++++++++++++++++-----
>  include/linux/srcu.h     |    6 +++---
>  2 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/notifier.h b/include/linux/notifier.h
> index d65746e..6bfd703 100644
> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -42,9 +42,7 @@
>   * in srcu_notifier_call_chain(): no cache bounces and no memory barriers.
>   * As compensation, srcu_notifier_chain_unregister() is rather expensive.
>   * SRCU notifier chains should be used when the chain will be called very
> - * often but notifier_blocks will seldom be removed.  Also, SRCU notifier
> - * chains are slightly more difficult to use because they require special
> - * runtime initialization.
> + * often but notifier_blocks will seldom be removed.
>   */
>  
>  struct notifier_block {
> @@ -85,7 +83,7 @@ struct srcu_notifier_head {
>  		(name)->head = NULL;		\
>  	} while (0)
>  
> -/* srcu_notifier_heads must be initialized and cleaned up dynamically */
> +/* srcu_notifier_heads must be cleaned up dynamically */
>  extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
>  #define srcu_cleanup_notifier_head(name)	\
>  		cleanup_srcu_struct(&(name)->srcu);
> @@ -98,7 +96,13 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
>  		.head = NULL }
>  #define RAW_NOTIFIER_INIT(name)	{				\
>  		.head = NULL }
> -/* srcu_notifier_heads cannot be initialized statically */
> +
> +#define SRCU_NOTIFIER_INIT(name, pcpu)				\
> +	{							\
> +		.mutex = __MUTEX_INITIALIZER(name.mutex),	\
> +		.head = NULL,					\
> +		.srcu = __SRCU_STRUCT_INIT(name.srcu, pcpu),	\
> +	}


Hi, Sebastian

I don't want to expose __SRCU_STRUCT_INIT(),
due to it has strong coupling with the percpu array.

I hope other structure which uses SRCU should use init_srcu_struct().

Thanks,
Lai

>  
>  #define ATOMIC_NOTIFIER_HEAD(name)				\
>  	struct atomic_notifier_head name =			\
> @@ -110,6 +114,18 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
>  	struct raw_notifier_head name =				\
>  		RAW_NOTIFIER_INIT(name)
>  
> +#define _SRCU_NOTIFIER_HEAD(name, mod)				\
> +	static DEFINE_PER_CPU(struct srcu_struct_array,		\
> +			name##_head_srcu_array);		\
> +	mod struct srcu_notifier_head name =			\
> +			SRCU_NOTIFIER_INIT(name, name##_head_srcu_array)
> +
> +#define SRCU_NOTIFIER_HEAD(name)				\
> +	_SRCU_NOTIFIER_HEAD(name, )
> +
> +#define SRCU_NOTIFIER_HEAD_STATIC(name)				\
> +	_SRCU_NOTIFIER_HEAD(name, static)
> +
>  #ifdef __KERNEL__
>  
>  extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index d04acb8..fe9efd4 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -84,10 +84,10 @@ int init_srcu_struct(struct srcu_struct *sp);
>  
>  void process_srcu(struct work_struct *work);
>  
> -#define __SRCU_STRUCT_INIT(name)					\
> +#define __SRCU_STRUCT_INIT(name, pcpu_name)				\
>  	{								\
>  		.completed = -300,					\
> -		.per_cpu_ref = &name##_srcu_array,			\
> +		.per_cpu_ref = &pcpu_name,				\
>  		.queue_lock = __SPIN_LOCK_UNLOCKED(name.queue_lock),	\
>  		.running = false,					\
>  		.batch_queue = RCU_BATCH_INIT(name.batch_queue),	\
> @@ -105,7 +105,7 @@ void process_srcu(struct work_struct *work);
>  #define _DEFINE_SRCU(name, mod)						\
>  	static DEFINE_PER_CPU(struct srcu_struct_array, name##_srcu_array);\
>  	mod struct srcu_struct name =					\
> -				__SRCU_STRUCT_INIT(name);
> +				__SRCU_STRUCT_INIT(name, name##_srcu_array);
>  
>  #define DEFINE_SRCU(name)		_DEFINE_SRCU(name, )
>  #define DEFINE_STATIC_SRCU(name)	_DEFINE_SRCU(name, static)

Re: [PATCH 2/3] kernel/SRCU: provide a static initializer

[Sebastian Andrzej Siewior]

On 04/05/2013 09:21 AM, Lai Jiangshan wrote:
> Hi, Sebastian

Hi Lai,

> I don't want to expose __SRCU_STRUCT_INIT(),
> due to it has strong coupling with the percpu array.
> 
> I hope other structure which uses SRCU should use init_srcu_struct().

I need a static initialization for this kind. Patch #3 shows one
example I have another one pending for crypto.
Do you have any idea how I could get it done without this? Do you want
to move/merge header files?

> 
> Thanks,
> Lai

Sebastian

Re: [PATCH 1/3] kernel/srcu: merge common code into a macro

[Sebastian Andrzej Siewior]

On 04/05/2013 09:21 AM, Lai Jiangshan wrote:
> Hi, Sebastian

Hi Lai,

> The patch hurts readability.
> The original code are simple enough, merging them as one macro
> gives us no benefit.

Except when you need to adjust the macro and you have to touch both of
them instead just one. But I don't have strong feelings for that one.

> Thanks
> Lai.

Sebastian

Re: [PATCH 2/3] kernel/SRCU: provide a static initializer

[Lai Jiangshan]

On 04/08/2013 06:03 PM, Sebastian Andrzej Siewior wrote:
> On 04/05/2013 09:21 AM, Lai Jiangshan wrote:
>> Hi, Sebastian
> 
> Hi Lai,
> 
>> I don't want to expose __SRCU_STRUCT_INIT(),
>> due to it has strong coupling with the percpu array.
>>
>> I hope other structure which uses SRCU should use init_srcu_struct().
> 
> I need a static initialization for this kind. Patch #3 shows one
> example I have another one pending for crypto.

If the percpu array can be defined in __SRCU_STRUCT_INIT(),
I'm happy to expose it. but it is not currently.

Why crypto can't use boot time initialization?

> Do you have any idea how I could get it done without this? Do you want
> to move/merge header files?

if crypto has to use static initialization, I will find out some way
or use your patch.

Thanks,
Lai

> 
>>
>> Thanks,
>> Lai
> 
> Sebastian
> 

Re: [PATCH 2/3] kernel/SRCU: provide a static initializer

[Sebastian Andrzej Siewior]

* Lai Jiangshan | 2013-04-09 09:09:56 [+0800]:

>If the percpu array can be defined in __SRCU_STRUCT_INIT(),
>I'm happy to expose it. but it is not currently.

I have no idea how to achieve this.

>Why crypto can't use boot time initialization?

It would require something like this:
--- linux-stable.orig/crypto/Kconfig
+++ linux-stable/crypto/Kconfig
@@ -13,7 +13,7 @@ source "crypto/async_tx/Kconfig"
 # Cryptographic API Configuration
 #
 menuconfig CRYPTO
-       tristate "Cryptographic API"
+       bool "Cryptographic API"
        help
          This option provides the core Cryptographic API.

--- linux-stable.orig/crypto/api.c
+++ linux-stable/crypto/api.c
@@ -34,6 +34,13 @@ EXPORT_SYMBOL_GPL(crypto_alg_sem);
 struct srcu_notifier_head crypto_chain;
 EXPORT_SYMBOL_GPL(crypto_chain);

+static int __init crypto_api_init(void)
+{
+       srcu_init_notifier_head(&crypto_chain);
+       return 0;
+}
+core_initcall(crypto_api_init);
+
 static inline struct crypto_alg *crypto_alg_get(struct crypto_alg *alg)
 {
        atomic_inc(&alg->cra_refcnt);

and there is no need for this.

>> Do you have any idea how I could get it done without this? Do you want
>> to move/merge header files?
>
>if crypto has to use static initialization, I will find out some way
>or use your patch.

The crypto would like this:

Subject: crypto: Convert crypto notifier chain to SRCU
From: Peter Zijlstra <peterz@infradead.org>
Date: Fri, 05 Oct 2012 09:03:24 +0100

The crypto notifier deadlocks on RT. Though this can be a real deadlock
on mainline as well due to fifo fair rwsems.

The involved parties here are:

[   82.172678] swapper/0       S 0000000000000001     0     1      0 0x00000000
[   82.172682]  ffff88042f18fcf0 0000000000000046 ffff88042f18fc80 ffffffff81491238
[   82.172685]  0000000000011cc0 0000000000011cc0 ffff88042f18c040 ffff88042f18ffd8
[   82.172688]  0000000000011cc0 0000000000011cc0 ffff88042f18ffd8 0000000000011cc0
[   82.172689] Call Trace:
[   82.172697]  [<ffffffff81491238>] ? _raw_spin_unlock_irqrestore+0x6c/0x7a
[   82.172701]  [<ffffffff8148fd3f>] schedule+0x64/0x66
[   82.172704]  [<ffffffff8148ec6b>] schedule_timeout+0x27/0xd0
[   82.172708]  [<ffffffff81043c0c>] ? unpin_current_cpu+0x1a/0x6c
[   82.172713]  [<ffffffff8106e491>] ? migrate_enable+0x12f/0x141
[   82.172716]  [<ffffffff8148fbbd>] wait_for_common+0xbb/0x11f
[   82.172719]  [<ffffffff810709f2>] ? try_to_wake_up+0x182/0x182
[   82.172722]  [<ffffffff8148fc96>] wait_for_completion_interruptible+0x1d/0x2e
[   82.172726]  [<ffffffff811debfd>] crypto_wait_for_test+0x49/0x6b
[   82.172728]  [<ffffffff811ded32>] crypto_register_alg+0x53/0x5a
[   82.172730]  [<ffffffff811ded6c>] crypto_register_algs+0x33/0x72
[   82.172734]  [<ffffffff81ad7686>] ? aes_init+0x12/0x12
[   82.172737]  [<ffffffff81ad76ea>] aesni_init+0x64/0x66
[   82.172741]  [<ffffffff81000318>] do_one_initcall+0x7f/0x13b
[   82.172744]  [<ffffffff81ac4d34>] kernel_init+0x199/0x22c
[   82.172747]  [<ffffffff81ac44ef>] ? loglevel+0x31/0x31
[   82.172752]  [<ffffffff814987c4>] kernel_thread_helper+0x4/0x10
[   82.172755]  [<ffffffff81491574>] ? retint_restore_args+0x13/0x13
[   82.172759]  [<ffffffff81ac4b9b>] ? start_kernel+0x3ca/0x3ca
[   82.172761]  [<ffffffff814987c0>] ? gs_change+0x13/0x13

[   82.174186] cryptomgr_test  S 0000000000000001     0    41      2 0x00000000
[   82.174189]  ffff88042c971980 0000000000000046 ffffffff81d74830 0000000000000292
[   82.174192]  0000000000011cc0 0000000000011cc0 ffff88042c96eb80 ffff88042c971fd8
[   82.174195]  0000000000011cc0 0000000000011cc0 ffff88042c971fd8 0000000000011cc0
[   82.174195] Call Trace:
[   82.174198]  [<ffffffff8148fd3f>] schedule+0x64/0x66
[   82.174201]  [<ffffffff8148ec6b>] schedule_timeout+0x27/0xd0
[   82.174204]  [<ffffffff81043c0c>] ? unpin_current_cpu+0x1a/0x6c
[   82.174206]  [<ffffffff8106e491>] ? migrate_enable+0x12f/0x141
[   82.174209]  [<ffffffff8148fbbd>] wait_for_common+0xbb/0x11f
[   82.174212]  [<ffffffff810709f2>] ? try_to_wake_up+0x182/0x182
[   82.174215]  [<ffffffff8148fc96>] wait_for_completion_interruptible+0x1d/0x2e
[   82.174218]  [<ffffffff811e4883>] cryptomgr_notify+0x280/0x385
[   82.174221]  [<ffffffff814943de>] notifier_call_chain+0x6b/0x98
[   82.174224]  [<ffffffff8108a11c>] ? rt_down_read+0x10/0x12
[   82.174227]  [<ffffffff810677cd>] __blocking_notifier_call_chain+0x70/0x8d
[   82.174230]  [<ffffffff810677fe>] blocking_notifier_call_chain+0x14/0x16
[   82.174234]  [<ffffffff811dd272>] crypto_probing_notify+0x24/0x50
[   82.174236]  [<ffffffff811dd7a1>] crypto_alg_mod_lookup+0x3e/0x74
[   82.174238]  [<ffffffff811dd949>] crypto_alloc_base+0x36/0x8f
[   82.174241]  [<ffffffff811e9408>] cryptd_alloc_ablkcipher+0x6e/0xb5
[   82.174243]  [<ffffffff811dd591>] ? kzalloc.clone.5+0xe/0x10
[   82.174246]  [<ffffffff8103085d>] ablk_init_common+0x1d/0x38
[   82.174249]  [<ffffffff8103852a>] ablk_ecb_init+0x15/0x17
[   82.174251]  [<ffffffff811dd8c6>] __crypto_alloc_tfm+0xc7/0x114
[   82.174254]  [<ffffffff811e0caa>] ? crypto_lookup_skcipher+0x1f/0xe4
[   82.174256]  [<ffffffff811e0dcf>] crypto_alloc_ablkcipher+0x60/0xa5
[   82.174258]  [<ffffffff811e5bde>] alg_test_skcipher+0x24/0x9b
[   82.174261]  [<ffffffff8106d96d>] ? finish_task_switch+0x3f/0xfa
[   82.174263]  [<ffffffff811e6b8e>] alg_test+0x16f/0x1d7
[   82.174267]  [<ffffffff811e45ac>] ? cryptomgr_probe+0xac/0xac
[   82.174269]  [<ffffffff811e45d8>] cryptomgr_test+0x2c/0x47
[   82.174272]  [<ffffffff81061161>] kthread+0x7e/0x86
[   82.174275]  [<ffffffff8106d9dd>] ? finish_task_switch+0xaf/0xfa
[   82.174278]  [<ffffffff814987c4>] kernel_thread_helper+0x4/0x10
[   82.174281]  [<ffffffff81491574>] ? retint_restore_args+0x13/0x13
[   82.174284]  [<ffffffff810610e3>] ? __init_kthread_worker+0x8c/0x8c
[   82.174287]  [<ffffffff814987c0>] ? gs_change+0x13/0x13

[   82.174329] cryptomgr_probe D 0000000000000002     0    47      2 0x00000000
[   82.174332]  ffff88042c991b70 0000000000000046 ffff88042c991bb0 0000000000000006
[   82.174335]  0000000000011cc0 0000000000011cc0 ffff88042c98ed00 ffff88042c991fd8
[   82.174338]  0000000000011cc0 0000000000011cc0 ffff88042c991fd8 0000000000011cc0
[   82.174338] Call Trace:
[   82.174342]  [<ffffffff8148fd3f>] schedule+0x64/0x66
[   82.174344]  [<ffffffff814901ad>] __rt_mutex_slowlock+0x85/0xbe
[   82.174347]  [<ffffffff814902d2>] rt_mutex_slowlock+0xec/0x159
[   82.174351]  [<ffffffff81089c4d>] rt_mutex_fastlock.clone.8+0x29/0x2f
[   82.174353]  [<ffffffff81490372>] rt_mutex_lock+0x33/0x37
[   82.174356]  [<ffffffff8108a0f2>] __rt_down_read+0x50/0x5a
[   82.174358]  [<ffffffff8108a11c>] ? rt_down_read+0x10/0x12
[   82.174360]  [<ffffffff8108a11c>] rt_down_read+0x10/0x12
[   82.174363]  [<ffffffff810677b5>] __blocking_notifier_call_chain+0x58/0x8d
[   82.174366]  [<ffffffff810677fe>] blocking_notifier_call_chain+0x14/0x16
[   82.174369]  [<ffffffff811dd272>] crypto_probing_notify+0x24/0x50
[   82.174372]  [<ffffffff811debd6>] crypto_wait_for_test+0x22/0x6b
[   82.174374]  [<ffffffff811decd3>] crypto_register_instance+0xb4/0xc0
[   82.174377]  [<ffffffff811e9b76>] cryptd_create+0x378/0x3b6
[   82.174379]  [<ffffffff811de512>] ? __crypto_lookup_template+0x5b/0x63
[   82.174382]  [<ffffffff811e4545>] cryptomgr_probe+0x45/0xac
[   82.174385]  [<ffffffff811e4500>] ? crypto_alloc_pcomp+0x1b/0x1b
[   82.174388]  [<ffffffff81061161>] kthread+0x7e/0x86
[   82.174391]  [<ffffffff8106d9dd>] ? finish_task_switch+0xaf/0xfa
[   82.174394]  [<ffffffff814987c4>] kernel_thread_helper+0x4/0x10
[   82.174398]  [<ffffffff81491574>] ? retint_restore_args+0x13/0x13
[   82.174401]  [<ffffffff810610e3>] ? __init_kthread_worker+0x8c/0x8c
[   82.174403]  [<ffffffff814987c0>] ? gs_change+0x13/0x13

cryptomgr_test spawns the cryptomgr_probe thread from the notifier
call. The probe thread fires the same notifier as the test thread and
deadlocks on the rwsem on RT.

Now this is a potential deadlock in mainline as well, because we have
fifo fair rwsems. If another thread blocks with a down_write() on the
notifier chain before the probe thread issues the down_read() it will
block the probe thread and the whole party is dead locked.

Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 crypto/algapi.c   |    4 ++--
 crypto/api.c      |    6 +++---
 crypto/internal.h |    4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -683,13 +683,13 @@ EXPORT_SYMBOL_GPL(crypto_spawn_tfm2);
 
 int crypto_register_notifier(struct notifier_block *nb)
 {
-	return blocking_notifier_chain_register(&crypto_chain, nb);
+	return srcu_notifier_chain_register(&crypto_chain, nb);
 }
 EXPORT_SYMBOL_GPL(crypto_register_notifier);
 
 int crypto_unregister_notifier(struct notifier_block *nb)
 {
-	return blocking_notifier_chain_unregister(&crypto_chain, nb);
+	return srcu_notifier_chain_unregister(&crypto_chain, nb);
 }
 EXPORT_SYMBOL_GPL(crypto_unregister_notifier);
 
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -31,7 +31,7 @@ EXPORT_SYMBOL_GPL(crypto_alg_list);
 DECLARE_RWSEM(crypto_alg_sem);
 EXPORT_SYMBOL_GPL(crypto_alg_sem);
 
-BLOCKING_NOTIFIER_HEAD(crypto_chain);
+SRCU_NOTIFIER_HEAD(crypto_chain);
 EXPORT_SYMBOL_GPL(crypto_chain);
 
 static inline struct crypto_alg *crypto_alg_get(struct crypto_alg *alg)
@@ -237,10 +237,10 @@ int crypto_probing_notify(unsigned long
 {
 	int ok;
 
-	ok = blocking_notifier_call_chain(&crypto_chain, val, v);
+	ok = srcu_notifier_call_chain(&crypto_chain, val, v);
 	if (ok == NOTIFY_DONE) {
 		request_module("cryptomgr");
-		ok = blocking_notifier_call_chain(&crypto_chain, val, v);
+		ok = srcu_notifier_call_chain(&crypto_chain, val, v);
 	}
 
 	return ok;
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -48,7 +48,7 @@ struct crypto_larval {
 
 extern struct list_head crypto_alg_list;
 extern struct rw_semaphore crypto_alg_sem;
-extern struct blocking_notifier_head crypto_chain;
+extern struct srcu_notifier_head crypto_chain;
 
 #ifdef CONFIG_PROC_FS
 void __init crypto_init_proc(void);
@@ -136,7 +136,7 @@ static inline int crypto_is_moribund(str
 
 static inline void crypto_notify(unsigned long val, void *v)
 {
-	blocking_notifier_call_chain(&crypto_chain, val, v);
+	srcu_notifier_call_chain(&crypto_chain, val, v);
 }
 
 #endif	/* _CRYPTO_INTERNAL_H */

>Thanks,
>Lai

Sebastian

Re: [PATCH 2/3] kernel/SRCU: provide a static initializer

[Lai Jiangshan]

On 04/12/2013 01:04 AM, Sebastian Andrzej Siewior wrote:
> * Lai Jiangshan | 2013-04-09 09:09:56 [+0800]:
> 
>> If the percpu array can be defined in __SRCU_STRUCT_INIT(),
>> I'm happy to expose it. but it is not currently.
> 
> I have no idea how to achieve this.
> 
>> Why crypto can't use boot time initialization?
> 
> It would require something like this:
> --- linux-stable.orig/crypto/Kconfig
> +++ linux-stable/crypto/Kconfig
> @@ -13,7 +13,7 @@ source "crypto/async_tx/Kconfig"
>  # Cryptographic API Configuration
>  #
>  menuconfig CRYPTO
> -       tristate "Cryptographic API"
> +       bool "Cryptographic API"
>         help
>           This option provides the core Cryptographic API.

Why convert to "bool"?
srcu_init_notifier_head() can be called in module-load-time.

> 
> --- linux-stable.orig/crypto/api.c
> +++ linux-stable/crypto/api.c
> @@ -34,6 +34,13 @@ EXPORT_SYMBOL_GPL(crypto_alg_sem);
>  struct srcu_notifier_head crypto_chain;
>  EXPORT_SYMBOL_GPL(crypto_chain);
> 
> +static int __init crypto_api_init(void)
> +{
> +       srcu_init_notifier_head(&crypto_chain);
> +       return 0;
> +}
> +core_initcall(crypto_api_init);
> +
>  static inline struct crypto_alg *crypto_alg_get(struct crypto_alg *alg)
>  {
>         atomic_inc(&alg->cra_refcnt);

And again, why crypto can't use boot time nor module-load-time initialization?

> 
> and there is no need for this.
> 
>>> Do you have any idea how I could get it done without this? Do you want
>>> to move/merge header files?
>>
>> if crypto has to use static initialization, I will find out some way
>> or use your patch.
> 
> The crypto would like this:
> 
> Subject: crypto: Convert crypto notifier chain to SRCU
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Fri, 05 Oct 2012 09:03:24 +0100
> 
> The crypto notifier deadlocks on RT. Though this can be a real deadlock
> on mainline as well due to fifo fair rwsems.
> 
> The involved parties here are:
> 
> [   82.172678] swapper/0       S 0000000000000001     0     1      0 0x00000000
> [   82.172682]  ffff88042f18fcf0 0000000000000046 ffff88042f18fc80 ffffffff81491238
> [   82.172685]  0000000000011cc0 0000000000011cc0 ffff88042f18c040 ffff88042f18ffd8
> [   82.172688]  0000000000011cc0 0000000000011cc0 ffff88042f18ffd8 0000000000011cc0
> [   82.172689] Call Trace:
> [   82.172697]  [<ffffffff81491238>] ? _raw_spin_unlock_irqrestore+0x6c/0x7a
> [   82.172701]  [<ffffffff8148fd3f>] schedule+0x64/0x66
> [   82.172704]  [<ffffffff8148ec6b>] schedule_timeout+0x27/0xd0
> [   82.172708]  [<ffffffff81043c0c>] ? unpin_current_cpu+0x1a/0x6c
> [   82.172713]  [<ffffffff8106e491>] ? migrate_enable+0x12f/0x141
> [   82.172716]  [<ffffffff8148fbbd>] wait_for_common+0xbb/0x11f
> [   82.172719]  [<ffffffff810709f2>] ? try_to_wake_up+0x182/0x182
> [   82.172722]  [<ffffffff8148fc96>] wait_for_completion_interruptible+0x1d/0x2e
> [   82.172726]  [<ffffffff811debfd>] crypto_wait_for_test+0x49/0x6b
> [   82.172728]  [<ffffffff811ded32>] crypto_register_alg+0x53/0x5a
> [   82.172730]  [<ffffffff811ded6c>] crypto_register_algs+0x33/0x72
> [   82.172734]  [<ffffffff81ad7686>] ? aes_init+0x12/0x12
> [   82.172737]  [<ffffffff81ad76ea>] aesni_init+0x64/0x66
> [   82.172741]  [<ffffffff81000318>] do_one_initcall+0x7f/0x13b
> [   82.172744]  [<ffffffff81ac4d34>] kernel_init+0x199/0x22c
> [   82.172747]  [<ffffffff81ac44ef>] ? loglevel+0x31/0x31
> [   82.172752]  [<ffffffff814987c4>] kernel_thread_helper+0x4/0x10
> [   82.172755]  [<ffffffff81491574>] ? retint_restore_args+0x13/0x13
> [   82.172759]  [<ffffffff81ac4b9b>] ? start_kernel+0x3ca/0x3ca
> [   82.172761]  [<ffffffff814987c0>] ? gs_change+0x13/0x13
> 
> [   82.174186] cryptomgr_test  S 0000000000000001     0    41      2 0x00000000
> [   82.174189]  ffff88042c971980 0000000000000046 ffffffff81d74830 0000000000000292
> [   82.174192]  0000000000011cc0 0000000000011cc0 ffff88042c96eb80 ffff88042c971fd8
> [   82.174195]  0000000000011cc0 0000000000011cc0 ffff88042c971fd8 0000000000011cc0
> [   82.174195] Call Trace:
> [   82.174198]  [<ffffffff8148fd3f>] schedule+0x64/0x66
> [   82.174201]  [<ffffffff8148ec6b>] schedule_timeout+0x27/0xd0
> [   82.174204]  [<ffffffff81043c0c>] ? unpin_current_cpu+0x1a/0x6c
> [   82.174206]  [<ffffffff8106e491>] ? migrate_enable+0x12f/0x141
> [   82.174209]  [<ffffffff8148fbbd>] wait_for_common+0xbb/0x11f
> [   82.174212]  [<ffffffff810709f2>] ? try_to_wake_up+0x182/0x182
> [   82.174215]  [<ffffffff8148fc96>] wait_for_completion_interruptible+0x1d/0x2e
> [   82.174218]  [<ffffffff811e4883>] cryptomgr_notify+0x280/0x385
> [   82.174221]  [<ffffffff814943de>] notifier_call_chain+0x6b/0x98
> [   82.174224]  [<ffffffff8108a11c>] ? rt_down_read+0x10/0x12
> [   82.174227]  [<ffffffff810677cd>] __blocking_notifier_call_chain+0x70/0x8d
> [   82.174230]  [<ffffffff810677fe>] blocking_notifier_call_chain+0x14/0x16
> [   82.174234]  [<ffffffff811dd272>] crypto_probing_notify+0x24/0x50
> [   82.174236]  [<ffffffff811dd7a1>] crypto_alg_mod_lookup+0x3e/0x74
> [   82.174238]  [<ffffffff811dd949>] crypto_alloc_base+0x36/0x8f
> [   82.174241]  [<ffffffff811e9408>] cryptd_alloc_ablkcipher+0x6e/0xb5
> [   82.174243]  [<ffffffff811dd591>] ? kzalloc.clone.5+0xe/0x10
> [   82.174246]  [<ffffffff8103085d>] ablk_init_common+0x1d/0x38
> [   82.174249]  [<ffffffff8103852a>] ablk_ecb_init+0x15/0x17
> [   82.174251]  [<ffffffff811dd8c6>] __crypto_alloc_tfm+0xc7/0x114
> [   82.174254]  [<ffffffff811e0caa>] ? crypto_lookup_skcipher+0x1f/0xe4
> [   82.174256]  [<ffffffff811e0dcf>] crypto_alloc_ablkcipher+0x60/0xa5
> [   82.174258]  [<ffffffff811e5bde>] alg_test_skcipher+0x24/0x9b
> [   82.174261]  [<ffffffff8106d96d>] ? finish_task_switch+0x3f/0xfa
> [   82.174263]  [<ffffffff811e6b8e>] alg_test+0x16f/0x1d7
> [   82.174267]  [<ffffffff811e45ac>] ? cryptomgr_probe+0xac/0xac
> [   82.174269]  [<ffffffff811e45d8>] cryptomgr_test+0x2c/0x47
> [   82.174272]  [<ffffffff81061161>] kthread+0x7e/0x86
> [   82.174275]  [<ffffffff8106d9dd>] ? finish_task_switch+0xaf/0xfa
> [   82.174278]  [<ffffffff814987c4>] kernel_thread_helper+0x4/0x10
> [   82.174281]  [<ffffffff81491574>] ? retint_restore_args+0x13/0x13
> [   82.174284]  [<ffffffff810610e3>] ? __init_kthread_worker+0x8c/0x8c
> [   82.174287]  [<ffffffff814987c0>] ? gs_change+0x13/0x13
> 
> [   82.174329] cryptomgr_probe D 0000000000000002     0    47      2 0x00000000
> [   82.174332]  ffff88042c991b70 0000000000000046 ffff88042c991bb0 0000000000000006
> [   82.174335]  0000000000011cc0 0000000000011cc0 ffff88042c98ed00 ffff88042c991fd8
> [   82.174338]  0000000000011cc0 0000000000011cc0 ffff88042c991fd8 0000000000011cc0
> [   82.174338] Call Trace:
> [   82.174342]  [<ffffffff8148fd3f>] schedule+0x64/0x66
> [   82.174344]  [<ffffffff814901ad>] __rt_mutex_slowlock+0x85/0xbe
> [   82.174347]  [<ffffffff814902d2>] rt_mutex_slowlock+0xec/0x159
> [   82.174351]  [<ffffffff81089c4d>] rt_mutex_fastlock.clone.8+0x29/0x2f
> [   82.174353]  [<ffffffff81490372>] rt_mutex_lock+0x33/0x37
> [   82.174356]  [<ffffffff8108a0f2>] __rt_down_read+0x50/0x5a
> [   82.174358]  [<ffffffff8108a11c>] ? rt_down_read+0x10/0x12
> [   82.174360]  [<ffffffff8108a11c>] rt_down_read+0x10/0x12
> [   82.174363]  [<ffffffff810677b5>] __blocking_notifier_call_chain+0x58/0x8d
> [   82.174366]  [<ffffffff810677fe>] blocking_notifier_call_chain+0x14/0x16
> [   82.174369]  [<ffffffff811dd272>] crypto_probing_notify+0x24/0x50
> [   82.174372]  [<ffffffff811debd6>] crypto_wait_for_test+0x22/0x6b
> [   82.174374]  [<ffffffff811decd3>] crypto_register_instance+0xb4/0xc0
> [   82.174377]  [<ffffffff811e9b76>] cryptd_create+0x378/0x3b6
> [   82.174379]  [<ffffffff811de512>] ? __crypto_lookup_template+0x5b/0x63
> [   82.174382]  [<ffffffff811e4545>] cryptomgr_probe+0x45/0xac
> [   82.174385]  [<ffffffff811e4500>] ? crypto_alloc_pcomp+0x1b/0x1b
> [   82.174388]  [<ffffffff81061161>] kthread+0x7e/0x86
> [   82.174391]  [<ffffffff8106d9dd>] ? finish_task_switch+0xaf/0xfa
> [   82.174394]  [<ffffffff814987c4>] kernel_thread_helper+0x4/0x10
> [   82.174398]  [<ffffffff81491574>] ? retint_restore_args+0x13/0x13
> [   82.174401]  [<ffffffff810610e3>] ? __init_kthread_worker+0x8c/0x8c
> [   82.174403]  [<ffffffff814987c0>] ? gs_change+0x13/0x13
> 
> cryptomgr_test spawns the cryptomgr_probe thread from the notifier
> call. The probe thread fires the same notifier as the test thread and
> deadlocks on the rwsem on RT.
> 
> Now this is a potential deadlock in mainline as well, because we have
> fifo fair rwsems. If another thread blocks with a down_write() on the
> notifier chain before the probe thread issues the down_read() it will
> block the probe thread and the whole party is dead locked.
> 
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  crypto/algapi.c   |    4 ++--
>  crypto/api.c      |    6 +++---
>  crypto/internal.h |    4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
> @@ -683,13 +683,13 @@ EXPORT_SYMBOL_GPL(crypto_spawn_tfm2);
>  
>  int crypto_register_notifier(struct notifier_block *nb)
>  {
> -	return blocking_notifier_chain_register(&crypto_chain, nb);
> +	return srcu_notifier_chain_register(&crypto_chain, nb);
>  }
>  EXPORT_SYMBOL_GPL(crypto_register_notifier);
>  
>  int crypto_unregister_notifier(struct notifier_block *nb)
>  {
> -	return blocking_notifier_chain_unregister(&crypto_chain, nb);
> +	return srcu_notifier_chain_unregister(&crypto_chain, nb);
>  }
>  EXPORT_SYMBOL_GPL(crypto_unregister_notifier);
>  
> --- a/crypto/api.c
> +++ b/crypto/api.c
> @@ -31,7 +31,7 @@ EXPORT_SYMBOL_GPL(crypto_alg_list);
>  DECLARE_RWSEM(crypto_alg_sem);
>  EXPORT_SYMBOL_GPL(crypto_alg_sem);
>  
> -BLOCKING_NOTIFIER_HEAD(crypto_chain);
> +SRCU_NOTIFIER_HEAD(crypto_chain);
>  EXPORT_SYMBOL_GPL(crypto_chain);
>  
>  static inline struct crypto_alg *crypto_alg_get(struct crypto_alg *alg)
> @@ -237,10 +237,10 @@ int crypto_probing_notify(unsigned long
>  {
>  	int ok;
>  
> -	ok = blocking_notifier_call_chain(&crypto_chain, val, v);
> +	ok = srcu_notifier_call_chain(&crypto_chain, val, v);
>  	if (ok == NOTIFY_DONE) {
>  		request_module("cryptomgr");
> -		ok = blocking_notifier_call_chain(&crypto_chain, val, v);
> +		ok = srcu_notifier_call_chain(&crypto_chain, val, v);
>  	}
>  
>  	return ok;
> --- a/crypto/internal.h
> +++ b/crypto/internal.h
> @@ -48,7 +48,7 @@ struct crypto_larval {
>  
>  extern struct list_head crypto_alg_list;
>  extern struct rw_semaphore crypto_alg_sem;
> -extern struct blocking_notifier_head crypto_chain;
> +extern struct srcu_notifier_head crypto_chain;
>  
>  #ifdef CONFIG_PROC_FS
>  void __init crypto_init_proc(void);
> @@ -136,7 +136,7 @@ static inline int crypto_is_moribund(str
>  
>  static inline void crypto_notify(unsigned long val, void *v)
>  {
> -	blocking_notifier_call_chain(&crypto_chain, val, v);
> +	srcu_notifier_call_chain(&crypto_chain, val, v);
>  }
>  
>  #endif	/* _CRYPTO_INTERNAL_H */
> 
>> Thanks,
>> Lai
> 
> Sebastian
> 

Re: [PATCH 2/3] kernel/SRCU: provide a static initializer

[Paul E. McKenney]

On Fri, Apr 12, 2013 at 10:56:27AM +0800, Lai Jiangshan wrote:
> On 04/12/2013 01:04 AM, Sebastian Andrzej Siewior wrote:
> > * Lai Jiangshan | 2013-04-09 09:09:56 [+0800]:
> > 
> >> If the percpu array can be defined in __SRCU_STRUCT_INIT(),
> >> I'm happy to expose it. but it is not currently.
> > 
> > I have no idea how to achieve this.
> > 
> >> Why crypto can't use boot time initialization?
> > 
> > It would require something like this:
> > --- linux-stable.orig/crypto/Kconfig
> > +++ linux-stable/crypto/Kconfig
> > @@ -13,7 +13,7 @@ source "crypto/async_tx/Kconfig"
> >  # Cryptographic API Configuration
> >  #
> >  menuconfig CRYPTO
> > -       tristate "Cryptographic API"
> > +       bool "Cryptographic API"
> >         help
> >           This option provides the core Cryptographic API.
> 
> Why convert to "bool"?
> srcu_init_notifier_head() can be called in module-load-time.
> 
> > 
> > --- linux-stable.orig/crypto/api.c
> > +++ linux-stable/crypto/api.c
> > @@ -34,6 +34,13 @@ EXPORT_SYMBOL_GPL(crypto_alg_sem);
> >  struct srcu_notifier_head crypto_chain;
> >  EXPORT_SYMBOL_GPL(crypto_chain);
> > 
> > +static int __init crypto_api_init(void)
> > +{
> > +       srcu_init_notifier_head(&crypto_chain);
> > +       return 0;
> > +}
> > +core_initcall(crypto_api_init);
> > +
> >  static inline struct crypto_alg *crypto_alg_get(struct crypto_alg *alg)
> >  {
> >         atomic_inc(&alg->cra_refcnt);
> 
> And again, why crypto can't use boot time nor module-load-time initialization?

Hello, Lai,

I believe that Sebastian's point is that compile-time initialization
would be easier to use and would reduce the amount of code that must
be written, reviewed, and executed.

							Thanx, Paul

> > and there is no need for this.
> > 
> >>> Do you have any idea how I could get it done without this? Do you want
> >>> to move/merge header files?
> >>
> >> if crypto has to use static initialization, I will find out some way
> >> or use your patch.
> > 
> > The crypto would like this:
> > 
> > Subject: crypto: Convert crypto notifier chain to SRCU
> > From: Peter Zijlstra <peterz@infradead.org>
> > Date: Fri, 05 Oct 2012 09:03:24 +0100
> > 
> > The crypto notifier deadlocks on RT. Though this can be a real deadlock
> > on mainline as well due to fifo fair rwsems.
> > 
> > The involved parties here are:
> > 
> > [   82.172678] swapper/0       S 0000000000000001     0     1      0 0x00000000
> > [   82.172682]  ffff88042f18fcf0 0000000000000046 ffff88042f18fc80 ffffffff81491238
> > [   82.172685]  0000000000011cc0 0000000000011cc0 ffff88042f18c040 ffff88042f18ffd8
> > [   82.172688]  0000000000011cc0 0000000000011cc0 ffff88042f18ffd8 0000000000011cc0
> > [   82.172689] Call Trace:
> > [   82.172697]  [<ffffffff81491238>] ? _raw_spin_unlock_irqrestore+0x6c/0x7a
> > [   82.172701]  [<ffffffff8148fd3f>] schedule+0x64/0x66
> > [   82.172704]  [<ffffffff8148ec6b>] schedule_timeout+0x27/0xd0
> > [   82.172708]  [<ffffffff81043c0c>] ? unpin_current_cpu+0x1a/0x6c
> > [   82.172713]  [<ffffffff8106e491>] ? migrate_enable+0x12f/0x141
> > [   82.172716]  [<ffffffff8148fbbd>] wait_for_common+0xbb/0x11f
> > [   82.172719]  [<ffffffff810709f2>] ? try_to_wake_up+0x182/0x182
> > [   82.172722]  [<ffffffff8148fc96>] wait_for_completion_interruptible+0x1d/0x2e
> > [   82.172726]  [<ffffffff811debfd>] crypto_wait_for_test+0x49/0x6b
> > [   82.172728]  [<ffffffff811ded32>] crypto_register_alg+0x53/0x5a
> > [   82.172730]  [<ffffffff811ded6c>] crypto_register_algs+0x33/0x72
> > [   82.172734]  [<ffffffff81ad7686>] ? aes_init+0x12/0x12
> > [   82.172737]  [<ffffffff81ad76ea>] aesni_init+0x64/0x66
> > [   82.172741]  [<ffffffff81000318>] do_one_initcall+0x7f/0x13b
> > [   82.172744]  [<ffffffff81ac4d34>] kernel_init+0x199/0x22c
> > [   82.172747]  [<ffffffff81ac44ef>] ? loglevel+0x31/0x31
> > [   82.172752]  [<ffffffff814987c4>] kernel_thread_helper+0x4/0x10
> > [   82.172755]  [<ffffffff81491574>] ? retint_restore_args+0x13/0x13
> > [   82.172759]  [<ffffffff81ac4b9b>] ? start_kernel+0x3ca/0x3ca
> > [   82.172761]  [<ffffffff814987c0>] ? gs_change+0x13/0x13
> > 
> > [   82.174186] cryptomgr_test  S 0000000000000001     0    41      2 0x00000000
> > [   82.174189]  ffff88042c971980 0000000000000046 ffffffff81d74830 0000000000000292
> > [   82.174192]  0000000000011cc0 0000000000011cc0 ffff88042c96eb80 ffff88042c971fd8
> > [   82.174195]  0000000000011cc0 0000000000011cc0 ffff88042c971fd8 0000000000011cc0
> > [   82.174195] Call Trace:
> > [   82.174198]  [<ffffffff8148fd3f>] schedule+0x64/0x66
> > [   82.174201]  [<ffffffff8148ec6b>] schedule_timeout+0x27/0xd0
> > [   82.174204]  [<ffffffff81043c0c>] ? unpin_current_cpu+0x1a/0x6c
> > [   82.174206]  [<ffffffff8106e491>] ? migrate_enable+0x12f/0x141
> > [   82.174209]  [<ffffffff8148fbbd>] wait_for_common+0xbb/0x11f
> > [   82.174212]  [<ffffffff810709f2>] ? try_to_wake_up+0x182/0x182
> > [   82.174215]  [<ffffffff8148fc96>] wait_for_completion_interruptible+0x1d/0x2e
> > [   82.174218]  [<ffffffff811e4883>] cryptomgr_notify+0x280/0x385
> > [   82.174221]  [<ffffffff814943de>] notifier_call_chain+0x6b/0x98
> > [   82.174224]  [<ffffffff8108a11c>] ? rt_down_read+0x10/0x12
> > [   82.174227]  [<ffffffff810677cd>] __blocking_notifier_call_chain+0x70/0x8d
> > [   82.174230]  [<ffffffff810677fe>] blocking_notifier_call_chain+0x14/0x16
> > [   82.174234]  [<ffffffff811dd272>] crypto_probing_notify+0x24/0x50
> > [   82.174236]  [<ffffffff811dd7a1>] crypto_alg_mod_lookup+0x3e/0x74
> > [   82.174238]  [<ffffffff811dd949>] crypto_alloc_base+0x36/0x8f
> > [   82.174241]  [<ffffffff811e9408>] cryptd_alloc_ablkcipher+0x6e/0xb5
> > [   82.174243]  [<ffffffff811dd591>] ? kzalloc.clone.5+0xe/0x10
> > [   82.174246]  [<ffffffff8103085d>] ablk_init_common+0x1d/0x38
> > [   82.174249]  [<ffffffff8103852a>] ablk_ecb_init+0x15/0x17
> > [   82.174251]  [<ffffffff811dd8c6>] __crypto_alloc_tfm+0xc7/0x114
> > [   82.174254]  [<ffffffff811e0caa>] ? crypto_lookup_skcipher+0x1f/0xe4
> > [   82.174256]  [<ffffffff811e0dcf>] crypto_alloc_ablkcipher+0x60/0xa5
> > [   82.174258]  [<ffffffff811e5bde>] alg_test_skcipher+0x24/0x9b
> > [   82.174261]  [<ffffffff8106d96d>] ? finish_task_switch+0x3f/0xfa
> > [   82.174263]  [<ffffffff811e6b8e>] alg_test+0x16f/0x1d7
> > [   82.174267]  [<ffffffff811e45ac>] ? cryptomgr_probe+0xac/0xac
> > [   82.174269]  [<ffffffff811e45d8>] cryptomgr_test+0x2c/0x47
> > [   82.174272]  [<ffffffff81061161>] kthread+0x7e/0x86
> > [   82.174275]  [<ffffffff8106d9dd>] ? finish_task_switch+0xaf/0xfa
> > [   82.174278]  [<ffffffff814987c4>] kernel_thread_helper+0x4/0x10
> > [   82.174281]  [<ffffffff81491574>] ? retint_restore_args+0x13/0x13
> > [   82.174284]  [<ffffffff810610e3>] ? __init_kthread_worker+0x8c/0x8c
> > [   82.174287]  [<ffffffff814987c0>] ? gs_change+0x13/0x13
> > 
> > [   82.174329] cryptomgr_probe D 0000000000000002     0    47      2 0x00000000
> > [   82.174332]  ffff88042c991b70 0000000000000046 ffff88042c991bb0 0000000000000006
> > [   82.174335]  0000000000011cc0 0000000000011cc0 ffff88042c98ed00 ffff88042c991fd8
> > [   82.174338]  0000000000011cc0 0000000000011cc0 ffff88042c991fd8 0000000000011cc0
> > [   82.174338] Call Trace:
> > [   82.174342]  [<ffffffff8148fd3f>] schedule+0x64/0x66
> > [   82.174344]  [<ffffffff814901ad>] __rt_mutex_slowlock+0x85/0xbe
> > [   82.174347]  [<ffffffff814902d2>] rt_mutex_slowlock+0xec/0x159
> > [   82.174351]  [<ffffffff81089c4d>] rt_mutex_fastlock.clone.8+0x29/0x2f
> > [   82.174353]  [<ffffffff81490372>] rt_mutex_lock+0x33/0x37
> > [   82.174356]  [<ffffffff8108a0f2>] __rt_down_read+0x50/0x5a
> > [   82.174358]  [<ffffffff8108a11c>] ? rt_down_read+0x10/0x12
> > [   82.174360]  [<ffffffff8108a11c>] rt_down_read+0x10/0x12
> > [   82.174363]  [<ffffffff810677b5>] __blocking_notifier_call_chain+0x58/0x8d
> > [   82.174366]  [<ffffffff810677fe>] blocking_notifier_call_chain+0x14/0x16
> > [   82.174369]  [<ffffffff811dd272>] crypto_probing_notify+0x24/0x50
> > [   82.174372]  [<ffffffff811debd6>] crypto_wait_for_test+0x22/0x6b
> > [   82.174374]  [<ffffffff811decd3>] crypto_register_instance+0xb4/0xc0
> > [   82.174377]  [<ffffffff811e9b76>] cryptd_create+0x378/0x3b6
> > [   82.174379]  [<ffffffff811de512>] ? __crypto_lookup_template+0x5b/0x63
> > [   82.174382]  [<ffffffff811e4545>] cryptomgr_probe+0x45/0xac
> > [   82.174385]  [<ffffffff811e4500>] ? crypto_alloc_pcomp+0x1b/0x1b
> > [   82.174388]  [<ffffffff81061161>] kthread+0x7e/0x86
> > [   82.174391]  [<ffffffff8106d9dd>] ? finish_task_switch+0xaf/0xfa
> > [   82.174394]  [<ffffffff814987c4>] kernel_thread_helper+0x4/0x10
> > [   82.174398]  [<ffffffff81491574>] ? retint_restore_args+0x13/0x13
> > [   82.174401]  [<ffffffff810610e3>] ? __init_kthread_worker+0x8c/0x8c
> > [   82.174403]  [<ffffffff814987c0>] ? gs_change+0x13/0x13
> > 
> > cryptomgr_test spawns the cryptomgr_probe thread from the notifier
> > call. The probe thread fires the same notifier as the test thread and
> > deadlocks on the rwsem on RT.
> > 
> > Now this is a potential deadlock in mainline as well, because we have
> > fifo fair rwsems. If another thread blocks with a down_write() on the
> > notifier chain before the probe thread issues the down_read() it will
> > block the probe thread and the whole party is dead locked.
> > 
> > Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > ---
> >  crypto/algapi.c   |    4 ++--
> >  crypto/api.c      |    6 +++---
> >  crypto/internal.h |    4 ++--
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > --- a/crypto/algapi.c
> > +++ b/crypto/algapi.c
> > @@ -683,13 +683,13 @@ EXPORT_SYMBOL_GPL(crypto_spawn_tfm2);
> >  
> >  int crypto_register_notifier(struct notifier_block *nb)
> >  {
> > -	return blocking_notifier_chain_register(&crypto_chain, nb);
> > +	return srcu_notifier_chain_register(&crypto_chain, nb);
> >  }
> >  EXPORT_SYMBOL_GPL(crypto_register_notifier);
> >  
> >  int crypto_unregister_notifier(struct notifier_block *nb)
> >  {
> > -	return blocking_notifier_chain_unregister(&crypto_chain, nb);
> > +	return srcu_notifier_chain_unregister(&crypto_chain, nb);
> >  }
> >  EXPORT_SYMBOL_GPL(crypto_unregister_notifier);
> >  
> > --- a/crypto/api.c
> > +++ b/crypto/api.c
> > @@ -31,7 +31,7 @@ EXPORT_SYMBOL_GPL(crypto_alg_list);
> >  DECLARE_RWSEM(crypto_alg_sem);
> >  EXPORT_SYMBOL_GPL(crypto_alg_sem);
> >  
> > -BLOCKING_NOTIFIER_HEAD(crypto_chain);
> > +SRCU_NOTIFIER_HEAD(crypto_chain);
> >  EXPORT_SYMBOL_GPL(crypto_chain);
> >  
> >  static inline struct crypto_alg *crypto_alg_get(struct crypto_alg *alg)
> > @@ -237,10 +237,10 @@ int crypto_probing_notify(unsigned long
> >  {
> >  	int ok;
> >  
> > -	ok = blocking_notifier_call_chain(&crypto_chain, val, v);
> > +	ok = srcu_notifier_call_chain(&crypto_chain, val, v);
> >  	if (ok == NOTIFY_DONE) {
> >  		request_module("cryptomgr");
> > -		ok = blocking_notifier_call_chain(&crypto_chain, val, v);
> > +		ok = srcu_notifier_call_chain(&crypto_chain, val, v);
> >  	}
> >  
> >  	return ok;
> > --- a/crypto/internal.h
> > +++ b/crypto/internal.h
> > @@ -48,7 +48,7 @@ struct crypto_larval {
> >  
> >  extern struct list_head crypto_alg_list;
> >  extern struct rw_semaphore crypto_alg_sem;
> > -extern struct blocking_notifier_head crypto_chain;
> > +extern struct srcu_notifier_head crypto_chain;
> >  
> >  #ifdef CONFIG_PROC_FS
> >  void __init crypto_init_proc(void);
> > @@ -136,7 +136,7 @@ static inline int crypto_is_moribund(str
> >  
> >  static inline void crypto_notify(unsigned long val, void *v)
> >  {
> > -	blocking_notifier_call_chain(&crypto_chain, val, v);
> > +	srcu_notifier_call_chain(&crypto_chain, val, v);
> >  }
> >  
> >  #endif	/* _CRYPTO_INTERNAL_H */
> > 
> >> Thanks,
> >> Lai
> > 
> > Sebastian
> > 
>