[4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[Luis Claudio R. Goncalves]

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

Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[Thomas Gleixner]

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.

Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[Luis Claudio R. Goncalves]

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

Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[Josh Cartwright]

[-- 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 --]

Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[Luis Claudio R. Goncalves]

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 ]

Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[Sebastian Andrzej Siewior]

* 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

Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[Josh Cartwright]

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

Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[Josh Cartwright]

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

Re: [4.4.4-rt11] Possiblie recursive locking detected in kswapd / mb_cache_shrink_scan()

[Sebastian Andrzej Siewior]

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

[PATCH] list_bl: fixup bogus lockdep warning

[Josh Cartwright]

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

Re: [PATCH] list_bl: fixup bogus lockdep warning

[Sebastian Andrzej Siewior]

* 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

Re: [PATCH] list_bl: fixup bogus lockdep warning

[Sebastian Andrzej Siewior]

* 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

Re: [PATCH] list_bl: fixup bogus lockdep warning

[Josh Cartwright]

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