* [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() @ 2016-03-14 21:00 Luis Claudio R. Goncalves 2016-03-19 8:33 ` Thomas Gleixner 0 siblings, 1 reply; 13+ messages in thread From: Luis Claudio R. Goncalves @ 2016-03-14 21:00 UTC (permalink / raw) To: linux-rt-users Hello, I just started using 4.4.4-rt11 this afternoon and saw the lockdep splat below. Is this something new or did I miss a patch floating on the list? [67547.917898] ============================================= [67547.917898] [ INFO: possible recursive locking detected ] [67547.917900] 4.4.4-rt11 #4 Not tainted [67547.917900] --------------------------------------------- [67547.917901] kswapd0/59 is trying to acquire lock: [67547.917908] (&h->lock#2){+.+.-.}, at: [<ffffffff812934dc>] mb_cache_shrink_scan+0xac/0x220 [67547.917908] but task is already holding lock: [67547.917910] (&h->lock#2){+.+.-.}, at: [<ffffffff812934ca>] mb_cache_shrink_scan+0x9a/0x220 [67547.917911] other info that might help us debug this: [67547.917911] Possible unsafe locking scenario: [67547.917911] CPU0 [67547.917911] ---- [67547.917912] lock(&h->lock#2); [67547.917913] lock(&h->lock#2); [67547.917913] *** DEADLOCK *** [67547.917913] May be due to missing lock nesting notation [67547.917914] 2 locks held by kswapd0/59: [67547.917917] #0: (shrinker_rwsem){+.+...}, at: [<ffffffff810e77d2>] rt_down_read_trylock+0x22/0x30 [67547.917919] #1: (&h->lock#2){+.+.-.}, at: [<ffffffff812934ca>] mb_cache_shrink_scan+0x9a/0x220 [67547.917920] stack backtrace: [67547.917921] CPU: 0 PID: 59 Comm: kswapd0 Not tainted 4.4.4-rt11 #4 [67547.917922] Hardware name: Hewlett-Packard p7-1512/2ADA, BIOS 8.15 02/05/2013 [67547.917923] 0000000000000086 000000005fca2e71 ffff88018a9fba10 ffffffff813d5412 [67547.917925] ffffffff828ede50 ffff88018a9f4000 ffff88018a9fbb00 ffffffff810e0d66 [67547.917926] ffff88018a9fbb08 000000000000008a ffffffff828e58a0 ffffffff81c0bf18 [67547.917926] Call Trace: [67547.917929] [<ffffffff813d5412>] dump_stack+0x67/0x95 [67547.917931] [<ffffffff810e0d66>] __lock_acquire+0x1cf6/0x1d30 [67547.917934] [<ffffffff813f55d7>] ? debug_smp_processor_id+0x17/0x20 [67547.917936] [<ffffffff810deb49>] ? mark_held_locks+0x79/0xa0 [67547.917940] [<ffffffff817dc0cc>] ? _raw_spin_unlock_irqrestore+0x6c/0x80 [67547.917941] [<ffffffff810e1907>] lock_acquire+0xf7/0x240 [67547.917942] [<ffffffff812934dc>] ? mb_cache_shrink_scan+0xac/0x220 [67547.917944] [<ffffffff817dbb11>] _raw_spin_lock+0x41/0x80 [67547.917945] [<ffffffff812934dc>] ? mb_cache_shrink_scan+0xac/0x220 [67547.917946] [<ffffffff812934dc>] mb_cache_shrink_scan+0xac/0x220 [67547.917949] [<ffffffff811bfaaa>] shrink_slab.part.41+0x24a/0x600 [67547.917951] [<ffffffff811c479a>] shrink_zone+0x2ca/0x2e0 [67547.917953] [<ffffffff811c5ee1>] kswapd+0x581/0xbc0 [67547.917955] [<ffffffff811c5960>] ? mem_cgroup_shrink_node_zone+0x3c0/0x3c0 [67547.917957] [<ffffffff810a9b71>] kthread+0x101/0x120 [67547.917959] [<ffffffff810a9a70>] ? kthread_create_on_node+0x250/0x250 [67547.917960] [<ffffffff817dcfff>] ret_from_fork+0x3f/0x70 [67547.917962] [<ffffffff810a9a70>] ? kthread_create_on_node+0x250/0x250 Best regards, Luis ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() 2016-03-14 21:00 [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() Luis Claudio R. Goncalves @ 2016-03-19 8:33 ` Thomas Gleixner 2016-03-21 13:34 ` Luis Claudio R. Goncalves 2016-03-21 14:40 ` Josh Cartwright 0 siblings, 2 replies; 13+ messages in thread From: Thomas Gleixner @ 2016-03-19 8:33 UTC (permalink / raw) To: Luis Claudio R. Goncalves; +Cc: linux-rt-users On Mon, 14 Mar 2016, Luis Claudio R. Goncalves wrote: > Hello, > > I just started using 4.4.4-rt11 this afternoon and saw the lockdep splat below. > Is this something new or did I miss a patch floating on the list? That's new. Looking into it. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() 2016-03-19 8:33 ` Thomas Gleixner @ 2016-03-21 13:34 ` Luis Claudio R. Goncalves 2016-03-21 14:40 ` Josh Cartwright 1 sibling, 0 replies; 13+ messages in thread From: Luis Claudio R. Goncalves @ 2016-03-21 13:34 UTC (permalink / raw) To: Thomas Gleixner; +Cc: linux-rt-users On Sat, Mar 19, 2016 at 09:33:34AM +0100, Thomas Gleixner wrote: | On Mon, 14 Mar 2016, Luis Claudio R. Goncalves wrote: | | > Hello, | > | > I just started using 4.4.4-rt11 this afternoon and saw the lockdep splat below. | > Is this something new or did I miss a patch floating on the list? | | That's new. Looking into it. Thanks! Please let me know if you want me to run any test or trace. I will be more than glad on helping. Luis ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() 2016-03-19 8:33 ` Thomas Gleixner 2016-03-21 13:34 ` Luis Claudio R. Goncalves @ 2016-03-21 14:40 ` Josh Cartwright 2016-03-21 20:47 ` Luis Claudio R. Goncalves 2016-03-29 15:04 ` Sebastian Andrzej Siewior 1 sibling, 2 replies; 13+ messages in thread From: Josh Cartwright @ 2016-03-21 14:40 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Luis Claudio R. Goncalves, linux-rt-users [-- Attachment #1: Type: text/plain, Size: 1514 bytes --] On Sat, Mar 19, 2016 at 09:33:34AM +0100, Thomas Gleixner wrote: > On Mon, 14 Mar 2016, Luis Claudio R. Goncalves wrote: > > > Hello, > > > > I just started using 4.4.4-rt11 this afternoon and saw the lockdep splat below. > > Is this something new or did I miss a patch floating on the list? > > That's new. Looking into it. It looks like the way that INIT_HLIST_BL_HEAD() is authored, all instances of the contained raw spinlock will share the same lockdep class. That, coupled with the following in mb_cache_shrink(), is triggering the lockdep violation: void mb_cache_shrink(struct block_device *bdev) { [..] /* * Prevent any find or get operation on the entry. */ hlist_bl_lock(ce->e_block_hash_p); hlist_bl_lock(ce->e_index_hash_p); Josh diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h index 44f0b55..d13428a 100644 --- a/include/linux/list_bl.h +++ b/include/linux/list_bl.h @@ -42,13 +42,18 @@ struct hlist_bl_node { struct hlist_bl_node *next, **pprev; }; -static inline void INIT_HLIST_BL_HEAD(struct hlist_bl_head *h) -{ - h->first = NULL; #ifdef CONFIG_PREEMPT_RT_BASE - raw_spin_lock_init(&h->lock); +#define INIT_HLIST_BL_HEAD(h) \ +do { \ + (h)->first = NULL; \ + raw_spin_lock_init(&(h)->lock); \ +} while (0) +#else +#define INIT_HLIST_BL_HEAD(h) \ +do { \ + (h)->first = NULL; \ +} while (0) #endif -} static inline void INIT_HLIST_BL_NODE(struct hlist_bl_node *h) { [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() 2016-03-21 14:40 ` Josh Cartwright @ 2016-03-21 20:47 ` Luis Claudio R. Goncalves 2016-03-29 15:04 ` Sebastian Andrzej Siewior 1 sibling, 0 replies; 13+ messages in thread From: Luis Claudio R. Goncalves @ 2016-03-21 20:47 UTC (permalink / raw) To: Josh Cartwright; +Cc: Thomas Gleixner, linux-rt-users On Mon, Mar 21, 2016 at 09:40:16AM -0500, Josh Cartwright wrote: | On Sat, Mar 19, 2016 at 09:33:34AM +0100, Thomas Gleixner wrote: | > On Mon, 14 Mar 2016, Luis Claudio R. Goncalves wrote: | > | > > Hello, | > > | > > I just started using 4.4.4-rt11 this afternoon and saw the lockdep splat below. | > > Is this something new or did I miss a patch floating on the list? | > | > That's new. Looking into it. | | It looks like the way that INIT_HLIST_BL_HEAD() is authored, all | instances of the contained raw spinlock will share the same lockdep | class. That, coupled with the following in mb_cache_shrink(), is | triggering the lockdep violation: | | void | mb_cache_shrink(struct block_device *bdev) | { | [..] | /* | * Prevent any find or get operation on the entry. | */ | hlist_bl_lock(ce->e_block_hash_p); | hlist_bl_lock(ce->e_index_hash_p); | | Josh Josh, I built a kernel with your patch and the lockdep complaint is gone. I was used to see the message within 30 minutes of normal usage. So far, four hours with that kernel running and no complaints. Tested-by: Luis Claudio R. Goncalves <lclaudio@uudg.org> Thanks! Luis | diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h | index 44f0b55..d13428a 100644 | --- a/include/linux/list_bl.h | +++ b/include/linux/list_bl.h | @@ -42,13 +42,18 @@ struct hlist_bl_node { | struct hlist_bl_node *next, **pprev; | }; | | -static inline void INIT_HLIST_BL_HEAD(struct hlist_bl_head *h) | -{ | - h->first = NULL; | #ifdef CONFIG_PREEMPT_RT_BASE | - raw_spin_lock_init(&h->lock); | +#define INIT_HLIST_BL_HEAD(h) \ | +do { \ | + (h)->first = NULL; \ | + raw_spin_lock_init(&(h)->lock); \ | +} while (0) | +#else | +#define INIT_HLIST_BL_HEAD(h) \ | +do { \ | + (h)->first = NULL; \ | +} while (0) | #endif | -} | | static inline void INIT_HLIST_BL_NODE(struct hlist_bl_node *h) | { ---end quoted text--- -- [ Luis Claudio R. Goncalves Bass - Gospel - RT ] [ Fingerprint: 4FDD B8C4 3C59 34BD 8BE9 2696 7203 D980 A448 C8F8 ] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() 2016-03-21 14:40 ` Josh Cartwright 2016-03-21 20:47 ` Luis Claudio R. Goncalves @ 2016-03-29 15:04 ` Sebastian Andrzej Siewior 2016-03-29 18:20 ` Josh Cartwright 1 sibling, 1 reply; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2016-03-29 15:04 UTC (permalink / raw) To: Josh Cartwright Cc: Thomas Gleixner, Luis Claudio R. Goncalves, linux-rt-users * Josh Cartwright | 2016-03-21 09:40:16 [-0500]: >diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h >index 44f0b55..d13428a 100644 >--- a/include/linux/list_bl.h >+++ b/include/linux/list_bl.h >@@ -42,13 +42,18 @@ struct hlist_bl_node { > struct hlist_bl_node *next, **pprev; > }; > >-static inline void INIT_HLIST_BL_HEAD(struct hlist_bl_head *h) >-{ >- h->first = NULL; > #ifdef CONFIG_PREEMPT_RT_BASE >- raw_spin_lock_init(&h->lock); >+#define INIT_HLIST_BL_HEAD(h) \ >+do { \ >+ (h)->first = NULL; \ >+ raw_spin_lock_init(&(h)->lock); \ >+} while (0) >+#else >+#define INIT_HLIST_BL_HEAD(h) \ >+do { \ >+ (h)->first = NULL; \ >+} while (0) > #endif >-} So we use a macro instead a "static inline" to ensure we end up with another lockdep class? This surprises me because it would mean that the function wasn't inlined / key classed was re-used. Looking at the assembly I see: |ffffffff8121fb41: 48 c7 c2 e0 bb d6 82 mov $0xffffffff82d6bbe0,%rdx |ffffffff8121fb48: 48 89 df mov %rbx,%rdi |ffffffff8121fb4b: 48 c7 c6 30 2b a2 81 mov $0xffffffff81a22b30,%rsi |ffffffff8121fb52: e8 a9 0e e9 ff callq ffffffff810b0a00 <__raw_spin_lock_init> … |ffffffff8121fc12: 48 c7 c2 d0 bb d6 82 mov $0xffffffff82d6bbd0,%rdx |ffffffff8121fc19: 48 c7 c6 7b 2e a3 81 mov $0xffffffff81a32e7b,%rsi |ffffffff8121fc20: 48 c7 07 00 00 00 00 movq $0x0,(%rdi) |ffffffff8121fc27: 48 83 c7 08 add $0x8,%rdi |ffffffff8121fc2b: e8 d0 0d e9 ff callq ffffffff810b0a00 <__raw_spin_lock_init> rdx holds the third parameter and it is different. What do I miss? Was the rdx argument in Luis' case the same before this path was applied? Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() 2016-03-29 15:04 ` Sebastian Andrzej Siewior @ 2016-03-29 18:20 ` Josh Cartwright 2016-03-29 23:14 ` Josh Cartwright 0 siblings, 1 reply; 13+ messages in thread From: Josh Cartwright @ 2016-03-29 18:20 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Thomas Gleixner, Luis Claudio R. Goncalves, linux-rt-users On Tue, Mar 29, 2016 at 05:04:44PM +0200, Sebastian Andrzej Siewior wrote: > * Josh Cartwright | 2016-03-21 09:40:16 [-0500]: > > >diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h > >index 44f0b55..d13428a 100644 > >--- a/include/linux/list_bl.h > >+++ b/include/linux/list_bl.h > >@@ -42,13 +42,18 @@ struct hlist_bl_node { > > struct hlist_bl_node *next, **pprev; > > }; > > > >-static inline void INIT_HLIST_BL_HEAD(struct hlist_bl_head *h) > >-{ > >- h->first = NULL; > > #ifdef CONFIG_PREEMPT_RT_BASE > >- raw_spin_lock_init(&h->lock); > >+#define INIT_HLIST_BL_HEAD(h) \ > >+do { \ > >+ (h)->first = NULL; \ > >+ raw_spin_lock_init(&(h)->lock); \ > >+} while (0) > >+#else > >+#define INIT_HLIST_BL_HEAD(h) \ > >+do { \ > >+ (h)->first = NULL; \ > >+} while (0) > > #endif > >-} > > So we use a macro instead a "static inline" to ensure we end up with > another lockdep class? Yes, that was the idea. > This surprises me because it would mean that the function wasn't > inlined / key classed was re-used. Hmm. I'm not sure that's what this means. I think it's possible that the function _code_ is inlined, but any function static data is shared between inlined instances. This simple experiment seems to also confirm this: extern void do_something(int *x); static inline void experiment_static(void) { static int x; do_something(&x); } #define experiment_macro() \ do { \ static int x; \ do_something(&x); \ } while (0) void f(void) { asm("# static1"); experiment_static(); asm("# static2"); experiment_static(); asm("# macro1"); experiment_macro(); asm("# macro2"); experiment_macro(); } Generated listing (cleaned up): f: subq $8, %rsp # static1 movl $x.1835, %edi call do_something # static2 movl $x.1835, %edi call do_something # macro1 movl $x.1839, %edi call do_something # macro2 movl $x.1840, %edi addq $8, %rsp jmp do_something [..] .local x.1835 .comm x.1835,4,4 .local x.1840 .comm x.1840,4,4 .local x.1839 .comm x.1839,4,4 .ident "GCC: (GNU) 5.3.0" You can see here that both of the static inlined instances shared the same 'x.1835' data (and the code is inlined), whereas the macro instances do not share data. > Looking at the assembly I see: > > |ffffffff8121fb41: 48 c7 c2 e0 bb d6 82 mov $0xffffffff82d6bbe0,%rdx > |ffffffff8121fb48: 48 89 df mov %rbx,%rdi > |ffffffff8121fb4b: 48 c7 c6 30 2b a2 81 mov $0xffffffff81a22b30,%rsi > |ffffffff8121fb52: e8 a9 0e e9 ff callq ffffffff810b0a00 <__raw_spin_lock_init> > ??? > |ffffffff8121fc12: 48 c7 c2 d0 bb d6 82 mov $0xffffffff82d6bbd0,%rdx > |ffffffff8121fc19: 48 c7 c6 7b 2e a3 81 mov $0xffffffff81a32e7b,%rsi > |ffffffff8121fc20: 48 c7 07 00 00 00 00 movq $0x0,(%rdi) > |ffffffff8121fc27: 48 83 c7 08 add $0x8,%rdi > |ffffffff8121fc2b: e8 d0 0d e9 ff callq ffffffff810b0a00 <__raw_spin_lock_init> > > rdx holds the third parameter and it is different. What do I miss? Hmm. I'm not sure what's going on here for your build. Here's the relevant listing I get for fs/mbcache.c w/ DEBUG_SPINLOCKS & PREEMPT_RT_FULL, which shows the same thing as the experiment above (the class object being shared amongst inline instances): mb_cache_create: [..] movq $__key.15124, %rdx #, movq $.LC5, %rsi #, movq $0, (%rdi) #, _28->first addq $8, %rdi #, D.27485 call __raw_spin_lock_init # [..] movq $__key.15124, %rdx #, movq $.LC5, %rsi #, movq $0, (%rdi) #, _35->first addq $8, %rdi #, D.27485 call __raw_spin_lock_init # [..] .local __key.15124 .comm __key.15124,8,8 Josh ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() 2016-03-29 18:20 ` Josh Cartwright @ 2016-03-29 23:14 ` Josh Cartwright 2016-03-30 8:16 ` Sebastian Andrzej Siewior 0 siblings, 1 reply; 13+ messages in thread From: Josh Cartwright @ 2016-03-29 23:14 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Thomas Gleixner, Luis Claudio R. Goncalves, linux-rt-users On Tue, Mar 29, 2016 at 01:20:02PM -0500, Josh Cartwright wrote: > On Tue, Mar 29, 2016 at 05:04:44PM +0200, Sebastian Andrzej Siewior wrote: [..] > > Looking at the assembly I see: > > > > |ffffffff8121fb41: 48 c7 c2 e0 bb d6 82 mov $0xffffffff82d6bbe0,%rdx > > |ffffffff8121fb48: 48 89 df mov %rbx,%rdi > > |ffffffff8121fb4b: 48 c7 c6 30 2b a2 81 mov $0xffffffff81a22b30,%rsi > > |ffffffff8121fb52: e8 a9 0e e9 ff callq ffffffff810b0a00 <__raw_spin_lock_init> > > ??? > > |ffffffff8121fc12: 48 c7 c2 d0 bb d6 82 mov $0xffffffff82d6bbd0,%rdx > > |ffffffff8121fc19: 48 c7 c6 7b 2e a3 81 mov $0xffffffff81a32e7b,%rsi > > |ffffffff8121fc20: 48 c7 07 00 00 00 00 movq $0x0,(%rdi) > > |ffffffff8121fc27: 48 83 c7 08 add $0x8,%rdi > > |ffffffff8121fc2b: e8 d0 0d e9 ff callq ffffffff810b0a00 <__raw_spin_lock_init> > > > > rdx holds the third parameter and it is different. What do I miss? > > Hmm. I'm not sure what's going on here for your build. Ah! I think the problem is a bit more nuanced than I let on before. The problem isn't that _all_ instances of the hlist_bl_head will share a lockdep class...it's that instances which are INIT_HLIST_BL_HEAD()'d _within the same compilation unit_ will share a lockdep class. Depending on what you are looking at, perhaps that's what you're seeing? Josh ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() 2016-03-29 23:14 ` Josh Cartwright @ 2016-03-30 8:16 ` Sebastian Andrzej Siewior 2016-03-31 5:04 ` [PATCH] list_bl: fixup bogus lockdep warning Josh Cartwright 0 siblings, 1 reply; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2016-03-30 8:16 UTC (permalink / raw) To: Josh Cartwright Cc: Thomas Gleixner, Luis Claudio R. Goncalves, linux-rt-users On 03/30/2016 01:14 AM, Josh Cartwright wrote: > On Tue, Mar 29, 2016 at 01:20:02PM -0500, Josh Cartwright wrote: >> On Tue, Mar 29, 2016 at 05:04:44PM +0200, Sebastian Andrzej Siewior wrote: > > The problem isn't that _all_ instances of the hlist_bl_head will share a > lockdep class...it's that instances which are INIT_HLIST_BL_HEAD()'d _within > the same compilation unit_ will share a lockdep class. no, that is okay. No, that is actually all good. I must have mixed up the lock inits. Sorry for the confusion. After looking at it again it is all good. Please send a proper patch and I add it to the queue. > > > Josh > Sebastian ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] list_bl: fixup bogus lockdep warning 2016-03-30 8:16 ` Sebastian Andrzej Siewior @ 2016-03-31 5:04 ` Josh Cartwright 2016-04-01 10:46 ` Sebastian Andrzej Siewior 2016-04-01 10:49 ` Sebastian Andrzej Siewior 0 siblings, 2 replies; 13+ messages in thread From: Josh Cartwright @ 2016-03-31 5:04 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: linux-rt-users, Thomas Gleixner, Luis Claudio R. Goncalves At first glance, the use of 'static inline' seems appropriate for INIT_HLIST_BL_HEAD(). However, when a 'static inline' function invocation is inlined by gcc, all callers share any static local data declared within that inline function. This presents a problem for how lockdep classes are setup. raw_spinlocks, for example, when CONFIG_DEBUG_SPINLOCK, # define raw_spin_lock_init(lock) \ do { \ static struct lock_class_key __key; \ \ __raw_spin_lock_init((lock), #lock, &__key); \ } while (0) When this macro is expanded into a 'static inline' caller, like INIT_HLIST_BL_HEAD(): static inline INIT_HLIST_BL_HEAD(struct hlist_bl_head *h) { h->first = NULL; raw_spin_lock_init(&h->lock); } ...the static local lock_class_key object is made a function static. For compilation units which initialize invoke INIT_HLIST_BL_HEAD() more than once, then, all of the invocations share this same static local object. This can lead to some very confusing lockdep splats (example below). Solve this problem by forcing the INIT_HLIST_BL_HEAD() to be a macro, which prevents the lockdep class object sharing. ============================================= [ INFO: possible recursive locking detected ] 4.4.4-rt11 #4 Not tainted --------------------------------------------- kswapd0/59 is trying to acquire lock: (&h->lock#2){+.+.-.}, at: mb_cache_shrink_scan but task is already holding lock: (&h->lock#2){+.+.-.}, at: mb_cache_shrink_scan other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&h->lock#2); lock(&h->lock#2); *** DEADLOCK *** May be due to missing lock nesting notation 2 locks held by kswapd0/59: #0: (shrinker_rwsem){+.+...}, at: rt_down_read_trylock #1: (&h->lock#2){+.+.-.}, at: mb_cache_shrink_scan Reported-by: Luis Claudio R. Goncalves <lclaudio@uudg.org> Tested-by: Luis Claudio R. Goncalves <lclaudio@uudg.org> Signed-off-by: Josh Cartwright <joshc@ni.com> --- include/linux/list_bl.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h index 44f0b55..a1bfb3b 100644 --- a/include/linux/list_bl.h +++ b/include/linux/list_bl.h @@ -42,13 +42,16 @@ struct hlist_bl_node { struct hlist_bl_node *next, **pprev; }; -static inline void INIT_HLIST_BL_HEAD(struct hlist_bl_head *h) -{ - h->first = NULL; #ifdef CONFIG_PREEMPT_RT_BASE - raw_spin_lock_init(&h->lock); +#define INIT_HLIST_BL_HEAD(h) \ +do { \ + (h)->first = NULL; \ + raw_spin_lock_init(&(h)->lock); \ +} while (0) +#else +#define INIT_HLIST_BL_HEAD(h) \ + (h)->first = NULL; \ #endif -} static inline void INIT_HLIST_BL_NODE(struct hlist_bl_node *h) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] list_bl: fixup bogus lockdep warning 2016-03-31 5:04 ` [PATCH] list_bl: fixup bogus lockdep warning Josh Cartwright @ 2016-04-01 10:46 ` Sebastian Andrzej Siewior 2016-04-01 10:49 ` Sebastian Andrzej Siewior 1 sibling, 0 replies; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2016-04-01 10:46 UTC (permalink / raw) To: Josh Cartwright Cc: linux-rt-users, Thomas Gleixner, Luis Claudio R. Goncalves * Josh Cartwright | 2016-03-31 00:04:25 [-0500]: >At first glance, the use of 'static inline' seems appropriate for >INIT_HLIST_BL_HEAD(). … > >Reported-by: Luis Claudio R. Goncalves <lclaudio@uudg.org> >Tested-by: Luis Claudio R. Goncalves <lclaudio@uudg.org> >Signed-off-by: Josh Cartwright <joshc@ni.com> Applied, thanks. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] list_bl: fixup bogus lockdep warning 2016-03-31 5:04 ` [PATCH] list_bl: fixup bogus lockdep warning Josh Cartwright 2016-04-01 10:46 ` Sebastian Andrzej Siewior @ 2016-04-01 10:49 ` Sebastian Andrzej Siewior 2016-04-01 17:02 ` Josh Cartwright 1 sibling, 1 reply; 13+ messages in thread From: Sebastian Andrzej Siewior @ 2016-04-01 10:49 UTC (permalink / raw) To: Josh Cartwright Cc: linux-rt-users, Thomas Gleixner, Luis Claudio R. Goncalves * Josh Cartwright | 2016-03-31 00:04:25 [-0500]: >diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h >index 44f0b55..a1bfb3b 100644 >--- a/include/linux/list_bl.h >+++ b/include/linux/list_bl.h >@@ -42,13 +42,16 @@ struct hlist_bl_node { > struct hlist_bl_node *next, **pprev; > }; > >-static inline void INIT_HLIST_BL_HEAD(struct hlist_bl_head *h) >-{ >- h->first = NULL; > #ifdef CONFIG_PREEMPT_RT_BASE >- raw_spin_lock_init(&h->lock); >+#define INIT_HLIST_BL_HEAD(h) \ >+do { \ >+ (h)->first = NULL; \ >+ raw_spin_lock_init(&(h)->lock); \ >+} while (0) >+#else >+#define INIT_HLIST_BL_HEAD(h) \ >+ (h)->first = NULL; \ I switched the else part to #define INIT_HLIST_BL_HEAD(h) (h)->first = NULL > #endif >-} Sebastian ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] list_bl: fixup bogus lockdep warning 2016-04-01 10:49 ` Sebastian Andrzej Siewior @ 2016-04-01 17:02 ` Josh Cartwright 0 siblings, 0 replies; 13+ messages in thread From: Josh Cartwright @ 2016-04-01 17:02 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: linux-rt-users, Thomas Gleixner, Luis Claudio R. Goncalves On Fri, Apr 01, 2016 at 12:49:46PM +0200, Sebastian Andrzej Siewior wrote: > * Josh Cartwright | 2016-03-31 00:04:25 [-0500]: > > >diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h > >index 44f0b55..a1bfb3b 100644 > >--- a/include/linux/list_bl.h > >+++ b/include/linux/list_bl.h > >@@ -42,13 +42,16 @@ struct hlist_bl_node { > > struct hlist_bl_node *next, **pprev; > > }; > > > >-static inline void INIT_HLIST_BL_HEAD(struct hlist_bl_head *h) > >-{ > >- h->first = NULL; > > #ifdef CONFIG_PREEMPT_RT_BASE > >- raw_spin_lock_init(&h->lock); > >+#define INIT_HLIST_BL_HEAD(h) \ > >+do { \ > >+ (h)->first = NULL; \ > >+ raw_spin_lock_init(&(h)->lock); \ > >+} while (0) > >+#else > >+#define INIT_HLIST_BL_HEAD(h) \ > >+ (h)->first = NULL; \ > > I switched the else part to > > #define INIT_HLIST_BL_HEAD(h) (h)->first = NULL That works, thanks. I had it wrapped it in do { } while(0);, but checkpatch complained. Anyway, I decided to do some grepping, and this particular case isn't the only one where there might be unintentional lockdep class sharing. I hope to follow up on this when I'm not traveling. Josh ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-04-01 17:02 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-14 21:00 [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan() Luis Claudio R. Goncalves 2016-03-19 8:33 ` Thomas Gleixner 2016-03-21 13:34 ` Luis Claudio R. Goncalves 2016-03-21 14:40 ` Josh Cartwright 2016-03-21 20:47 ` Luis Claudio R. Goncalves 2016-03-29 15:04 ` Sebastian Andrzej Siewior 2016-03-29 18:20 ` Josh Cartwright 2016-03-29 23:14 ` Josh Cartwright 2016-03-30 8:16 ` Sebastian Andrzej Siewior 2016-03-31 5:04 ` [PATCH] list_bl: fixup bogus lockdep warning Josh Cartwright 2016-04-01 10:46 ` Sebastian Andrzej Siewior 2016-04-01 10:49 ` Sebastian Andrzej Siewior 2016-04-01 17:02 ` Josh Cartwright
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.