Re: [PATCH] Fix lockdep false positive in add_full()

2014-02-03 Thread Paul E. McKenney
On Mon, Feb 03, 2014 at 03:31:40PM -0800, David Rientjes wrote:
> On Mon, 3 Feb 2014, Paul E. McKenney wrote:
> 
> > Hello!
> > 
> > The add_full() function currently has a lockdep_assert_held() requiring
> > that the kmem_cache_node structure's ->list_lock be held.  However,
> > this lock is not acquired by add_full()'s caller deactivate_slab()
> > in the full-node case unless debugging is enabled.  Because full nodes
> > are accessed only by debugging code, this state of affairs results in
> > lockdep false-positive splats like the following:
> > 
> > [   43.942868] WARNING: CPU: 0 PID: 698 at 
> > /home/paulmck/public_git/linux-rcu/mm/slub.c:1007 
> > deactivate_slab+0x509/0x720()
> > [   43.943016] Modules linked in:
> > [   43.943016] CPU: 0 PID: 698 Comm: torture_onoff Not tainted 3.14.0-rc1+ 
> > #1
> > [   43.943016] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
> > [   43.943016]  03ef 88001e3f5ba8 818952ec 
> > 0046
> > [   43.943016]   88001e3f5be8 81049517 
> > ea784e00
> > [   43.943016]   ea7a9000 0002 
> > 
> > [   43.943016] Call Trace:
> > [   43.943016]  [] dump_stack+0x46/0x58
> > [   43.943016]  [] warn_slowpath_common+0x87/0xb0
> > [   43.943016]  [] warn_slowpath_null+0x15/0x20
> > [   43.943016]  [] deactivate_slab+0x509/0x720
> > [   43.943016]  [] ? slab_cpuup_callback+0x3b/0x100
> > [   43.943016]  [] ? slab_cpuup_callback+0xd2/0x100
> > [   43.943016]  [] slab_cpuup_callback+0xa4/0x100
> > [   43.943016]  [] notifier_call_chain+0x54/0x110
> > [   43.943016]  [] __raw_notifier_call_chain+0x9/0x10
> > [   43.943016]  [] __cpu_notify+0x1b/0x30
> > [   43.943016]  [] cpu_notify_nofail+0x10/0x20
> > [   43.943016]  [] _cpu_down+0x10d/0x2e0
> > [   43.943016]  [] cpu_down+0x30/0x50
> > [   43.943016]  [] torture_onoff+0xd3/0x3c0
> > [   43.943016]  [] ? torture_onoff_stats+0x90/0x90
> > [   43.943016]  [] kthread+0xdf/0x100
> > [   43.943016]  [] ? _raw_spin_unlock_irq+0x2b/0x40
> > [   43.943016]  [] ? flush_kthread_worker+0x130/0x130
> > [   43.943016]  [] ret_from_fork+0x7c/0xb0
> > [   43.943016]  [] ? flush_kthread_worker+0x130/0x130
> > 
> > This commit therefore does the lockdep check only if debuggging is
> > enabled, thus avoiding the false positives.
> > 
> > Signed-off-by: Paul E. McKenney 
> 
> This was discussed in http://marc.info/?t=13914579132, what do you 
> think about the patch in that thread instead?

Looks fine to me!  I also tried it out and it avoided the splats, as noted
in my mail in the other thread, so please feel free to add my Tested-by.

Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Fix lockdep false positive in add_full()

2014-02-03 Thread David Rientjes
On Mon, 3 Feb 2014, Paul E. McKenney wrote:

> Hello!
> 
> The add_full() function currently has a lockdep_assert_held() requiring
> that the kmem_cache_node structure's ->list_lock be held.  However,
> this lock is not acquired by add_full()'s caller deactivate_slab()
> in the full-node case unless debugging is enabled.  Because full nodes
> are accessed only by debugging code, this state of affairs results in
> lockdep false-positive splats like the following:
> 
> [   43.942868] WARNING: CPU: 0 PID: 698 at 
> /home/paulmck/public_git/linux-rcu/mm/slub.c:1007 
> deactivate_slab+0x509/0x720()
> [   43.943016] Modules linked in:
> [   43.943016] CPU: 0 PID: 698 Comm: torture_onoff Not tainted 3.14.0-rc1+ #1
> [   43.943016] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
> [   43.943016]  03ef 88001e3f5ba8 818952ec 
> 0046
> [   43.943016]   88001e3f5be8 81049517 
> ea784e00
> [   43.943016]   ea7a9000 0002 
> 
> [   43.943016] Call Trace:
> [   43.943016]  [] dump_stack+0x46/0x58
> [   43.943016]  [] warn_slowpath_common+0x87/0xb0
> [   43.943016]  [] warn_slowpath_null+0x15/0x20
> [   43.943016]  [] deactivate_slab+0x509/0x720
> [   43.943016]  [] ? slab_cpuup_callback+0x3b/0x100
> [   43.943016]  [] ? slab_cpuup_callback+0xd2/0x100
> [   43.943016]  [] slab_cpuup_callback+0xa4/0x100
> [   43.943016]  [] notifier_call_chain+0x54/0x110
> [   43.943016]  [] __raw_notifier_call_chain+0x9/0x10
> [   43.943016]  [] __cpu_notify+0x1b/0x30
> [   43.943016]  [] cpu_notify_nofail+0x10/0x20
> [   43.943016]  [] _cpu_down+0x10d/0x2e0
> [   43.943016]  [] cpu_down+0x30/0x50
> [   43.943016]  [] torture_onoff+0xd3/0x3c0
> [   43.943016]  [] ? torture_onoff_stats+0x90/0x90
> [   43.943016]  [] kthread+0xdf/0x100
> [   43.943016]  [] ? _raw_spin_unlock_irq+0x2b/0x40
> [   43.943016]  [] ? flush_kthread_worker+0x130/0x130
> [   43.943016]  [] ret_from_fork+0x7c/0xb0
> [   43.943016]  [] ? flush_kthread_worker+0x130/0x130
> 
> This commit therefore does the lockdep check only if debuggging is
> enabled, thus avoiding the false positives.
> 
> Signed-off-by: Paul E. McKenney 

This was discussed in http://marc.info/?t=13914579132, what do you 
think about the patch in that thread instead?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Fix lockdep false positive in add_full()

2014-02-03 Thread Paul E. McKenney
Hello!

The add_full() function currently has a lockdep_assert_held() requiring
that the kmem_cache_node structure's ->list_lock be held.  However,
this lock is not acquired by add_full()'s caller deactivate_slab()
in the full-node case unless debugging is enabled.  Because full nodes
are accessed only by debugging code, this state of affairs results in
lockdep false-positive splats like the following:

[   43.942868] WARNING: CPU: 0 PID: 698 at 
/home/paulmck/public_git/linux-rcu/mm/slub.c:1007 deactivate_slab+0x509/0x720()
[   43.943016] Modules linked in:
[   43.943016] CPU: 0 PID: 698 Comm: torture_onoff Not tainted 3.14.0-rc1+ #1
[   43.943016] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[   43.943016]  03ef 88001e3f5ba8 818952ec 
0046
[   43.943016]   88001e3f5be8 81049517 
ea784e00
[   43.943016]   ea7a9000 0002 

[   43.943016] Call Trace:
[   43.943016]  [] dump_stack+0x46/0x58
[   43.943016]  [] warn_slowpath_common+0x87/0xb0
[   43.943016]  [] warn_slowpath_null+0x15/0x20
[   43.943016]  [] deactivate_slab+0x509/0x720
[   43.943016]  [] ? slab_cpuup_callback+0x3b/0x100
[   43.943016]  [] ? slab_cpuup_callback+0xd2/0x100
[   43.943016]  [] slab_cpuup_callback+0xa4/0x100
[   43.943016]  [] notifier_call_chain+0x54/0x110
[   43.943016]  [] __raw_notifier_call_chain+0x9/0x10
[   43.943016]  [] __cpu_notify+0x1b/0x30
[   43.943016]  [] cpu_notify_nofail+0x10/0x20
[   43.943016]  [] _cpu_down+0x10d/0x2e0
[   43.943016]  [] cpu_down+0x30/0x50
[   43.943016]  [] torture_onoff+0xd3/0x3c0
[   43.943016]  [] ? torture_onoff_stats+0x90/0x90
[   43.943016]  [] kthread+0xdf/0x100
[   43.943016]  [] ? _raw_spin_unlock_irq+0x2b/0x40
[   43.943016]  [] ? flush_kthread_worker+0x130/0x130
[   43.943016]  [] ret_from_fork+0x7c/0xb0
[   43.943016]  [] ? flush_kthread_worker+0x130/0x130

This commit therefore does the lockdep check only if debuggging is
enabled, thus avoiding the false positives.

Signed-off-by: Paul E. McKenney 
Cc: linux...@kvack.org
Cc: c...@linux-foundation.org
Cc: penb...@kernel.org
Cc: m...@selenic.com

diff --git a/mm/slub.c b/mm/slub.c
index 7e3e0458bce4..6fff4d980b7c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1004,7 +1004,8 @@ static inline void slab_free_hook(struct kmem_cache *s, 
void *x)
 static void add_full(struct kmem_cache *s,
struct kmem_cache_node *n, struct page *page)
 {
-   lockdep_assert_held(>list_lock);
+   if (kmem_cache_debug(s))
+   lockdep_assert_held(>list_lock);
 
if (!(s->flags & SLAB_STORE_USER))
return;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Fix lockdep false positive in add_full()

2014-02-03 Thread Paul E. McKenney
Hello!

The add_full() function currently has a lockdep_assert_held() requiring
that the kmem_cache_node structure's -list_lock be held.  However,
this lock is not acquired by add_full()'s caller deactivate_slab()
in the full-node case unless debugging is enabled.  Because full nodes
are accessed only by debugging code, this state of affairs results in
lockdep false-positive splats like the following:

[   43.942868] WARNING: CPU: 0 PID: 698 at 
/home/paulmck/public_git/linux-rcu/mm/slub.c:1007 deactivate_slab+0x509/0x720()
[   43.943016] Modules linked in:
[   43.943016] CPU: 0 PID: 698 Comm: torture_onoff Not tainted 3.14.0-rc1+ #1
[   43.943016] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[   43.943016]  03ef 88001e3f5ba8 818952ec 
0046
[   43.943016]   88001e3f5be8 81049517 
ea784e00
[   43.943016]   ea7a9000 0002 

[   43.943016] Call Trace:
[   43.943016]  [818952ec] dump_stack+0x46/0x58
[   43.943016]  [81049517] warn_slowpath_common+0x87/0xb0
[   43.943016]  [81049555] warn_slowpath_null+0x15/0x20
[   43.943016]  [8116e679] deactivate_slab+0x509/0x720
[   43.943016]  [8116eebb] ? slab_cpuup_callback+0x3b/0x100
[   43.943016]  [8116ef52] ? slab_cpuup_callback+0xd2/0x100
[   43.943016]  [8116ef24] slab_cpuup_callback+0xa4/0x100
[   43.943016]  [818a4c14] notifier_call_chain+0x54/0x110
[   43.943016]  [81075b79] __raw_notifier_call_chain+0x9/0x10
[   43.943016]  [8104963b] __cpu_notify+0x1b/0x30
[   43.943016]  [81049720] cpu_notify_nofail+0x10/0x20
[   43.943016]  [8188cc5d] _cpu_down+0x10d/0x2e0
[   43.943016]  [8188ce60] cpu_down+0x30/0x50
[   43.943016]  [811205f3] torture_onoff+0xd3/0x3c0
[   43.943016]  [81120520] ? torture_onoff_stats+0x90/0x90
[   43.943016]  [810710df] kthread+0xdf/0x100
[   43.943016]  [818a09cb] ? _raw_spin_unlock_irq+0x2b/0x40
[   43.943016]  [81071000] ? flush_kthread_worker+0x130/0x130
[   43.943016]  [818a983c] ret_from_fork+0x7c/0xb0
[   43.943016]  [81071000] ? flush_kthread_worker+0x130/0x130

This commit therefore does the lockdep check only if debuggging is
enabled, thus avoiding the false positives.

Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com
Cc: linux...@kvack.org
Cc: c...@linux-foundation.org
Cc: penb...@kernel.org
Cc: m...@selenic.com

diff --git a/mm/slub.c b/mm/slub.c
index 7e3e0458bce4..6fff4d980b7c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1004,7 +1004,8 @@ static inline void slab_free_hook(struct kmem_cache *s, 
void *x)
 static void add_full(struct kmem_cache *s,
struct kmem_cache_node *n, struct page *page)
 {
-   lockdep_assert_held(n-list_lock);
+   if (kmem_cache_debug(s))
+   lockdep_assert_held(n-list_lock);
 
if (!(s-flags  SLAB_STORE_USER))
return;

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Fix lockdep false positive in add_full()

2014-02-03 Thread David Rientjes
On Mon, 3 Feb 2014, Paul E. McKenney wrote:

 Hello!
 
 The add_full() function currently has a lockdep_assert_held() requiring
 that the kmem_cache_node structure's -list_lock be held.  However,
 this lock is not acquired by add_full()'s caller deactivate_slab()
 in the full-node case unless debugging is enabled.  Because full nodes
 are accessed only by debugging code, this state of affairs results in
 lockdep false-positive splats like the following:
 
 [   43.942868] WARNING: CPU: 0 PID: 698 at 
 /home/paulmck/public_git/linux-rcu/mm/slub.c:1007 
 deactivate_slab+0x509/0x720()
 [   43.943016] Modules linked in:
 [   43.943016] CPU: 0 PID: 698 Comm: torture_onoff Not tainted 3.14.0-rc1+ #1
 [   43.943016] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
 [   43.943016]  03ef 88001e3f5ba8 818952ec 
 0046
 [   43.943016]   88001e3f5be8 81049517 
 ea784e00
 [   43.943016]   ea7a9000 0002 
 
 [   43.943016] Call Trace:
 [   43.943016]  [818952ec] dump_stack+0x46/0x58
 [   43.943016]  [81049517] warn_slowpath_common+0x87/0xb0
 [   43.943016]  [81049555] warn_slowpath_null+0x15/0x20
 [   43.943016]  [8116e679] deactivate_slab+0x509/0x720
 [   43.943016]  [8116eebb] ? slab_cpuup_callback+0x3b/0x100
 [   43.943016]  [8116ef52] ? slab_cpuup_callback+0xd2/0x100
 [   43.943016]  [8116ef24] slab_cpuup_callback+0xa4/0x100
 [   43.943016]  [818a4c14] notifier_call_chain+0x54/0x110
 [   43.943016]  [81075b79] __raw_notifier_call_chain+0x9/0x10
 [   43.943016]  [8104963b] __cpu_notify+0x1b/0x30
 [   43.943016]  [81049720] cpu_notify_nofail+0x10/0x20
 [   43.943016]  [8188cc5d] _cpu_down+0x10d/0x2e0
 [   43.943016]  [8188ce60] cpu_down+0x30/0x50
 [   43.943016]  [811205f3] torture_onoff+0xd3/0x3c0
 [   43.943016]  [81120520] ? torture_onoff_stats+0x90/0x90
 [   43.943016]  [810710df] kthread+0xdf/0x100
 [   43.943016]  [818a09cb] ? _raw_spin_unlock_irq+0x2b/0x40
 [   43.943016]  [81071000] ? flush_kthread_worker+0x130/0x130
 [   43.943016]  [818a983c] ret_from_fork+0x7c/0xb0
 [   43.943016]  [81071000] ? flush_kthread_worker+0x130/0x130
 
 This commit therefore does the lockdep check only if debuggging is
 enabled, thus avoiding the false positives.
 
 Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com

This was discussed in http://marc.info/?t=13914579132, what do you 
think about the patch in that thread instead?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Fix lockdep false positive in add_full()

2014-02-03 Thread Paul E. McKenney
On Mon, Feb 03, 2014 at 03:31:40PM -0800, David Rientjes wrote:
 On Mon, 3 Feb 2014, Paul E. McKenney wrote:
 
  Hello!
  
  The add_full() function currently has a lockdep_assert_held() requiring
  that the kmem_cache_node structure's -list_lock be held.  However,
  this lock is not acquired by add_full()'s caller deactivate_slab()
  in the full-node case unless debugging is enabled.  Because full nodes
  are accessed only by debugging code, this state of affairs results in
  lockdep false-positive splats like the following:
  
  [   43.942868] WARNING: CPU: 0 PID: 698 at 
  /home/paulmck/public_git/linux-rcu/mm/slub.c:1007 
  deactivate_slab+0x509/0x720()
  [   43.943016] Modules linked in:
  [   43.943016] CPU: 0 PID: 698 Comm: torture_onoff Not tainted 3.14.0-rc1+ 
  #1
  [   43.943016] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
  [   43.943016]  03ef 88001e3f5ba8 818952ec 
  0046
  [   43.943016]   88001e3f5be8 81049517 
  ea784e00
  [   43.943016]   ea7a9000 0002 
  
  [   43.943016] Call Trace:
  [   43.943016]  [818952ec] dump_stack+0x46/0x58
  [   43.943016]  [81049517] warn_slowpath_common+0x87/0xb0
  [   43.943016]  [81049555] warn_slowpath_null+0x15/0x20
  [   43.943016]  [8116e679] deactivate_slab+0x509/0x720
  [   43.943016]  [8116eebb] ? slab_cpuup_callback+0x3b/0x100
  [   43.943016]  [8116ef52] ? slab_cpuup_callback+0xd2/0x100
  [   43.943016]  [8116ef24] slab_cpuup_callback+0xa4/0x100
  [   43.943016]  [818a4c14] notifier_call_chain+0x54/0x110
  [   43.943016]  [81075b79] __raw_notifier_call_chain+0x9/0x10
  [   43.943016]  [8104963b] __cpu_notify+0x1b/0x30
  [   43.943016]  [81049720] cpu_notify_nofail+0x10/0x20
  [   43.943016]  [8188cc5d] _cpu_down+0x10d/0x2e0
  [   43.943016]  [8188ce60] cpu_down+0x30/0x50
  [   43.943016]  [811205f3] torture_onoff+0xd3/0x3c0
  [   43.943016]  [81120520] ? torture_onoff_stats+0x90/0x90
  [   43.943016]  [810710df] kthread+0xdf/0x100
  [   43.943016]  [818a09cb] ? _raw_spin_unlock_irq+0x2b/0x40
  [   43.943016]  [81071000] ? flush_kthread_worker+0x130/0x130
  [   43.943016]  [818a983c] ret_from_fork+0x7c/0xb0
  [   43.943016]  [81071000] ? flush_kthread_worker+0x130/0x130
  
  This commit therefore does the lockdep check only if debuggging is
  enabled, thus avoiding the false positives.
  
  Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com
 
 This was discussed in http://marc.info/?t=13914579132, what do you 
 think about the patch in that thread instead?

Looks fine to me!  I also tried it out and it avoided the splats, as noted
in my mail in the other thread, so please feel free to add my Tested-by.

Thanx, Paul

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/