* [PATCH 1/3] kernel/srcu: merge common code into a macro
@ 2013-03-19 14:16 Sebastian Andrzej Siewior
2013-03-19 14:16 ` [PATCH 2/3] kernel/SRCU: provide a static initializer Sebastian Andrzej Siewior
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-03-19 14:16 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Lai Jiangshan, linux-kernel, tglx, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/3] kernel/SRCU: provide a static initializer 2013-03-19 14:16 [PATCH 1/3] kernel/srcu: merge common code into a macro Sebastian Andrzej Siewior @ 2013-03-19 14:16 ` Sebastian Andrzej Siewior 2013-04-05 7:21 ` Lai Jiangshan 2013-03-19 14:16 ` [PATCH 3/3] cpufreq: use static initializer for the SRCU notifier Sebastian Andrzej Siewior ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2013-03-19 14:16 UTC (permalink / raw) To: Paul E. McKenney Cc: Lai Jiangshan, linux-kernel, tglx, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] kernel/SRCU: provide a static initializer 2013-03-19 14:16 ` [PATCH 2/3] kernel/SRCU: provide a static initializer Sebastian Andrzej Siewior @ 2013-04-05 7:21 ` Lai Jiangshan 2013-04-08 10:03 ` Sebastian Andrzej Siewior 0 siblings, 1 reply; 13+ messages in thread From: Lai Jiangshan @ 2013-04-05 7:21 UTC (permalink / raw) To: Sebastian Andrzej Siewior; +Cc: Paul E. McKenney, linux-kernel, tglx 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) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] kernel/SRCU: provide a static initializer 2013-04-05 7:21 ` Lai Jiangshan @ 2013-04-08 10:03 ` Sebastian Andrzej Siewior 2013-04-09 1:09 ` Lai Jiangshan 0 siblings, 1 reply; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2013-04-08 10:03 UTC (permalink / raw) To: Lai Jiangshan; +Cc: Paul E. McKenney, linux-kernel, tglx 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] kernel/SRCU: provide a static initializer 2013-04-08 10:03 ` Sebastian Andrzej Siewior @ 2013-04-09 1:09 ` Lai Jiangshan 2013-04-11 17:04 ` Sebastian Andrzej Siewior 0 siblings, 1 reply; 13+ messages in thread From: Lai Jiangshan @ 2013-04-09 1:09 UTC (permalink / raw) To: Sebastian Andrzej Siewior; +Cc: Paul E. McKenney, linux-kernel, tglx 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] kernel/SRCU: provide a static initializer 2013-04-09 1:09 ` Lai Jiangshan @ 2013-04-11 17:04 ` Sebastian Andrzej Siewior 2013-04-12 2:56 ` Lai Jiangshan 0 siblings, 1 reply; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2013-04-11 17:04 UTC (permalink / raw) To: Lai Jiangshan Cc: Paul E. McKenney, linux-kernel, tglx, Peter Zijlstra, Herbert Xu * 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] kernel/SRCU: provide a static initializer 2013-04-11 17:04 ` Sebastian Andrzej Siewior @ 2013-04-12 2:56 ` Lai Jiangshan 2013-04-16 18:20 ` Paul E. McKenney 0 siblings, 1 reply; 13+ messages in thread From: Lai Jiangshan @ 2013-04-12 2:56 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Paul E. McKenney, linux-kernel, tglx, Peter Zijlstra, Herbert Xu 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] kernel/SRCU: provide a static initializer 2013-04-12 2:56 ` Lai Jiangshan @ 2013-04-16 18:20 ` Paul E. McKenney 0 siblings, 0 replies; 13+ messages in thread From: Paul E. McKenney @ 2013-04-16 18:20 UTC (permalink / raw) To: Lai Jiangshan Cc: Sebastian Andrzej Siewior, linux-kernel, tglx, Peter Zijlstra, Herbert Xu 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 > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] cpufreq: use static initializer for the SRCU notifier 2013-03-19 14:16 [PATCH 1/3] kernel/srcu: merge common code into a macro Sebastian Andrzej Siewior 2013-03-19 14:16 ` [PATCH 2/3] kernel/SRCU: provide a static initializer Sebastian Andrzej Siewior @ 2013-03-19 14:16 ` Sebastian Andrzej Siewior 2013-03-19 16:22 ` [PATCH 1/3] kernel/srcu: merge common code into a macro Joe Perches 2013-04-05 7:21 ` Lai Jiangshan 3 siblings, 0 replies; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2013-03-19 14:16 UTC (permalink / raw) To: Paul E. McKenney Cc: Lai Jiangshan, linux-kernel, tglx, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] kernel/srcu: merge common code into a macro 2013-03-19 14:16 [PATCH 1/3] kernel/srcu: merge common code into a macro Sebastian Andrzej Siewior 2013-03-19 14:16 ` [PATCH 2/3] kernel/SRCU: provide a static initializer Sebastian Andrzej Siewior 2013-03-19 14:16 ` [PATCH 3/3] cpufreq: use static initializer for the SRCU notifier Sebastian Andrzej Siewior @ 2013-03-19 16:22 ` Joe Perches 2013-03-19 18:15 ` Sebastian Andrzej Siewior 2013-04-05 7:21 ` Lai Jiangshan 3 siblings, 1 reply; 13+ messages in thread From: Joe Perches @ 2013-03-19 16:22 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Paul E. McKenney, Lai Jiangshan, linux-kernel, tglx 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] kernel/srcu: merge common code into a macro 2013-03-19 16:22 ` [PATCH 1/3] kernel/srcu: merge common code into a macro Joe Perches @ 2013-03-19 18:15 ` Sebastian Andrzej Siewior 0 siblings, 0 replies; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2013-03-19 18:15 UTC (permalink / raw) To: Joe Perches; +Cc: Paul E. McKenney, Lai Jiangshan, linux-kernel, tglx 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] kernel/srcu: merge common code into a macro 2013-03-19 14:16 [PATCH 1/3] kernel/srcu: merge common code into a macro Sebastian Andrzej Siewior ` (2 preceding siblings ...) 2013-03-19 16:22 ` [PATCH 1/3] kernel/srcu: merge common code into a macro Joe Perches @ 2013-04-05 7:21 ` Lai Jiangshan 2013-04-08 10:05 ` Sebastian Andrzej Siewior 3 siblings, 1 reply; 13+ messages in thread From: Lai Jiangshan @ 2013-04-05 7:21 UTC (permalink / raw) To: Sebastian Andrzej Siewior; +Cc: Paul E. McKenney, linux-kernel, tglx 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] kernel/srcu: merge common code into a macro 2013-04-05 7:21 ` Lai Jiangshan @ 2013-04-08 10:05 ` Sebastian Andrzej Siewior 0 siblings, 0 replies; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2013-04-08 10:05 UTC (permalink / raw) To: Lai Jiangshan; +Cc: Paul E. McKenney, linux-kernel, tglx 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-04-16 18:21 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-03-19 14:16 [PATCH 1/3] kernel/srcu: merge common code into a macro Sebastian Andrzej Siewior 2013-03-19 14:16 ` [PATCH 2/3] kernel/SRCU: provide a static initializer Sebastian Andrzej Siewior 2013-04-05 7:21 ` Lai Jiangshan 2013-04-08 10:03 ` Sebastian Andrzej Siewior 2013-04-09 1:09 ` Lai Jiangshan 2013-04-11 17:04 ` Sebastian Andrzej Siewior 2013-04-12 2:56 ` Lai Jiangshan 2013-04-16 18:20 ` Paul E. McKenney 2013-03-19 14:16 ` [PATCH 3/3] cpufreq: use static initializer for the SRCU notifier Sebastian Andrzej Siewior 2013-03-19 16:22 ` [PATCH 1/3] kernel/srcu: merge common code into a macro Joe Perches 2013-03-19 18:15 ` Sebastian Andrzej Siewior 2013-04-05 7:21 ` Lai Jiangshan 2013-04-08 10:05 ` Sebastian Andrzej Siewior
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.