lockdep recursive locking detected (rcu_kthread / __cache_free)

lockdep recursive locking detected (rcu_kthread / __cache_free)

[Sitsofe Wheeler]

Hi,

While running 3.1.0-rc8 the following lockdep warning (seemingly related
to RCU) was printed as the kernel was starting.


udev: starting version 151
udevd (263): /proc/263/oom_adj is deprecated, please use /proc/263/oom_score_adj instead.

=============================================
[ INFO: possible recursive locking detected ]
3.1.0-rc8-dirty #508
---------------------------------------------
rcu_kthread/6 is trying to acquire lock:
 (&(&parent->list_lock)->rlock){..-...}, at: [<b016fe11>] __cache_free+0x2dd/0x382

but task is already holding lock:
 (&(&parent->list_lock)->rlock){..-...}, at: [<b016fe11>] __cache_free+0x2dd/0x382

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(&(&parent->list_lock)->rlock);
  lock(&(&parent->list_lock)->rlock);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

1 lock held by rcu_kthread/6:
 #0:  (&(&parent->list_lock)->rlock){..-...}, at: [<b016fe11>] __cache_free+0x2dd/0x382

stack backtrace:
Pid: 6, comm: rcu_kthread Not tainted 3.1.0-rc8-dirty #508
Call Trace:
 [<b0144466>] __lock_acquire+0xb90/0xc0e
 [<b044c0c2>] ? _raw_spin_unlock_irqrestore+0x2f/0x46
 [<b0223ebc>] ? debug_object_active_state+0x94/0xbc
 [<b01315af>] ? rcuhead_fixup_activate+0x26/0x4c
 [<b01448be>] lock_acquire+0x5b/0x72
 [<b016fe11>] ? __cache_free+0x2dd/0x382
 [<b044bb22>] _raw_spin_lock+0x25/0x34
 [<b016fe11>] ? __cache_free+0x2dd/0x382
 [<b016fe11>] __cache_free+0x2dd/0x382
 [<b016ff5c>] kmem_cache_free+0x3e/0x5b
 [<b0170097>] slab_destroy+0x11e/0x126
 [<b0170184>] free_block+0xe5/0x112
 [<b016fe54>] __cache_free+0x320/0x382
 [<b01759a1>] ? file_free_rcu+0x32/0x39
 [<b016ff5c>] kmem_cache_free+0x3e/0x5b
 [<b01759a1>] file_free_rcu+0x32/0x39
 [<b014ca68>] rcu_process_callbacks+0x95/0xa8
 [<b014cb34>] rcu_kthread+0xb9/0xd2
 [<b013356c>] ? wake_up_bit+0x1b/0x1b
 [<b014ca7b>] ? rcu_process_callbacks+0xa8/0xa8
 [<b0133305>] kthread+0x6c/0x71
 [<b0133299>] ? __init_kthread_worker+0x42/0x42
 [<b044ce02>] kernel_thread_helper+0x6/0xd

-- 
Sitsofe | http://sucs.org/~sits/

Re: lockdep recursive locking detected (rcu_kthread / __cache_free)

[Paul E. McKenney]

On Mon, Oct 03, 2011 at 06:53:22PM +0100, Sitsofe Wheeler wrote:
> Hi,
> 
> While running 3.1.0-rc8 the following lockdep warning (seemingly related
> to RCU) was printed as the kernel was starting.
> 
> 
> udev: starting version 151
> udevd (263): /proc/263/oom_adj is deprecated, please use /proc/263/oom_score_adj instead.
> 
> =============================================
> [ INFO: possible recursive locking detected ]
> 3.1.0-rc8-dirty #508
> ---------------------------------------------
> rcu_kthread/6 is trying to acquire lock:
>  (&(&parent->list_lock)->rlock){..-...}, at: [<b016fe11>] __cache_free+0x2dd/0x382
> 
> but task is already holding lock:
>  (&(&parent->list_lock)->rlock){..-...}, at: [<b016fe11>] __cache_free+0x2dd/0x382
> 
> other info that might help us debug this:
>  Possible unsafe locking scenario:
> 
>        CPU0
>        ----
>   lock(&(&parent->list_lock)->rlock);
>   lock(&(&parent->list_lock)->rlock);
> 
>  *** DEADLOCK ***
> 
>  May be due to missing lock nesting notation
> 
> 1 lock held by rcu_kthread/6:
>  #0:  (&(&parent->list_lock)->rlock){..-...}, at: [<b016fe11>] __cache_free+0x2dd/0x382
> 
> stack backtrace:
> Pid: 6, comm: rcu_kthread Not tainted 3.1.0-rc8-dirty #508
> Call Trace:
>  [<b0144466>] __lock_acquire+0xb90/0xc0e
>  [<b044c0c2>] ? _raw_spin_unlock_irqrestore+0x2f/0x46
>  [<b0223ebc>] ? debug_object_active_state+0x94/0xbc
>  [<b01315af>] ? rcuhead_fixup_activate+0x26/0x4c
>  [<b01448be>] lock_acquire+0x5b/0x72
>  [<b016fe11>] ? __cache_free+0x2dd/0x382
>  [<b044bb22>] _raw_spin_lock+0x25/0x34
>  [<b016fe11>] ? __cache_free+0x2dd/0x382
>  [<b016fe11>] __cache_free+0x2dd/0x382
>  [<b016ff5c>] kmem_cache_free+0x3e/0x5b
>  [<b0170097>] slab_destroy+0x11e/0x126
>  [<b0170184>] free_block+0xe5/0x112
>  [<b016fe54>] __cache_free+0x320/0x382

The first lock was acquired here in an RCU callback.  The later lock that
lockdep complained about appears to have been acquired from a recursive
call to __cache_free(), with no help from RCU.  This looks to me like
one of the issues that arise from the slab allocator using itself to
allocate slab metadata.

So the allocator guys (added to CC) need to sort this one out.

							Thanx, Paul

>  [<b01759a1>] ? file_free_rcu+0x32/0x39
>  [<b016ff5c>] kmem_cache_free+0x3e/0x5b
>  [<b01759a1>] file_free_rcu+0x32/0x39
>  [<b014ca68>] rcu_process_callbacks+0x95/0xa8
>  [<b014cb34>] rcu_kthread+0xb9/0xd2
>  [<b013356c>] ? wake_up_bit+0x1b/0x1b
>  [<b014ca7b>] ? rcu_process_callbacks+0xa8/0xa8
>  [<b0133305>] kthread+0x6c/0x71
>  [<b0133299>] ? __init_kthread_worker+0x42/0x42
>  [<b044ce02>] kernel_thread_helper+0x6/0xd
> 
> -- 
> Sitsofe | http://sucs.org/~sits/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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: lockdep recursive locking detected (rcu_kthread / __cache_free)

[Christoph Lameter]

On Mon, 3 Oct 2011, Paul E. McKenney wrote:

> The first lock was acquired here in an RCU callback.  The later lock that
> lockdep complained about appears to have been acquired from a recursive
> call to __cache_free(), with no help from RCU.  This looks to me like
> one of the issues that arise from the slab allocator using itself to
> allocate slab metadata.

Right. However, this is a false positive since the slab cache with
the metadata is different from the slab caches with the slab data. The slab
cache with the metadata does not use itself any metadata slab caches.

Re: lockdep recursive locking detected (rcu_kthread / __cache_free)

[Peter Zijlstra]

On Mon, 2011-10-03 at 15:46 -0500, Christoph Lameter wrote:
> On Mon, 3 Oct 2011, Paul E. McKenney wrote:
> 
> > The first lock was acquired here in an RCU callback.  The later lock that
> > lockdep complained about appears to have been acquired from a recursive
> > call to __cache_free(), with no help from RCU.  This looks to me like
> > one of the issues that arise from the slab allocator using itself to
> > allocate slab metadata.
> 
> Right. However, this is a false positive since the slab cache with
> the metadata is different from the slab caches with the slab data. The slab
> cache with the metadata does not use itself any metadata slab caches.

Sure, but we're supposed to have annotated that.. see
init_node_lock_keys()

Re: lockdep recursive locking detected (rcu_kthread / __cache_free)

[Paul E. McKenney]

On Mon, Oct 03, 2011 at 03:46:11PM -0500, Christoph Lameter wrote:
> On Mon, 3 Oct 2011, Paul E. McKenney wrote:
> 
> > The first lock was acquired here in an RCU callback.  The later lock that
> > lockdep complained about appears to have been acquired from a recursive
> > call to __cache_free(), with no help from RCU.  This looks to me like
> > one of the issues that arise from the slab allocator using itself to
> > allocate slab metadata.
> 
> Right. However, this is a false positive since the slab cache with
> the metadata is different from the slab caches with the slab data. The slab
> cache with the metadata does not use itself any metadata slab caches.

Wouldn't it be possible to pass a new flag to the metadata slab caches
upon creation so that their locks could be placed in a separate lock
class?  Just allocate a separate lock_class_key structure for each such
lock in that case, and then use lockdep_set_class_and_name to associate
that structure with the corresponding lock.  I do this in kernel/rcutree.c
in order to allow the rcu_node tree's locks to nest properly.

							Thanx, Paul

Re: lockdep recursive locking detected (rcu_kthread / __cache_free)

[Christoph Lameter]

On Mon, 3 Oct 2011, Paul E. McKenney wrote:

> On Mon, Oct 03, 2011 at 03:46:11PM -0500, Christoph Lameter wrote:
> > On Mon, 3 Oct 2011, Paul E. McKenney wrote:
> >
> > > The first lock was acquired here in an RCU callback.  The later lock that
> > > lockdep complained about appears to have been acquired from a recursive
> > > call to __cache_free(), with no help from RCU.  This looks to me like
> > > one of the issues that arise from the slab allocator using itself to
> > > allocate slab metadata.
> >
> > Right. However, this is a false positive since the slab cache with
> > the metadata is different from the slab caches with the slab data. The slab
> > cache with the metadata does not use itself any metadata slab caches.
>
> Wouldn't it be possible to pass a new flag to the metadata slab caches
> upon creation so that their locks could be placed in a separate lock
> class?  Just allocate a separate lock_class_key structure for each such
> lock in that case, and then use lockdep_set_class_and_name to associate
> that structure with the corresponding lock.  I do this in kernel/rcutree.c
> in order to allow the rcu_node tree's locks to nest properly.

We could give the kmalloc array a different class from created slab
caches. That should have the desired effect.

But that seems to be already the case (looking at init_node_lock_keys).
Non OFF_SLAB caches seem to be getting a different lock class? Why is this
not working?

static void init_node_lock_keys(int q)
{
        struct cache_sizes *s = malloc_sizes;

        if (g_cpucache_up != FULL)
                return;

        for (s = malloc_sizes; s->cs_size != ULONG_MAX; s++) {
                struct kmem_list3 *l3;

                l3 = s->cs_cachep->nodelists[q];
                if (!l3 || OFF_SLAB(s->cs_cachep))
                        continue;

                slab_set_lock_classes(s->cs_cachep, &on_slab_l3_key,
                                &on_slab_alc_key, q);
        }
}

Re: lockdep recursive locking detected (rcu_kthread / __cache_free)

[Peter Zijlstra]

On Tue, 2011-10-04 at 09:28 -0500, Christoph Lameter wrote:
> On Mon, 3 Oct 2011, Paul E. McKenney wrote:
> 
> > On Mon, Oct 03, 2011 at 03:46:11PM -0500, Christoph Lameter wrote:
> > > On Mon, 3 Oct 2011, Paul E. McKenney wrote:
> > >
> > > > The first lock was acquired here in an RCU callback.  The later lock that
> > > > lockdep complained about appears to have been acquired from a recursive
> > > > call to __cache_free(), with no help from RCU.  This looks to me like
> > > > one of the issues that arise from the slab allocator using itself to
> > > > allocate slab metadata.
> > >
> > > Right. However, this is a false positive since the slab cache with
> > > the metadata is different from the slab caches with the slab data. The slab
> > > cache with the metadata does not use itself any metadata slab caches.
> >
> > Wouldn't it be possible to pass a new flag to the metadata slab caches
> > upon creation so that their locks could be placed in a separate lock
> > class?  Just allocate a separate lock_class_key structure for each such
> > lock in that case, and then use lockdep_set_class_and_name to associate
> > that structure with the corresponding lock.  I do this in kernel/rcutree.c
> > in order to allow the rcu_node tree's locks to nest properly.
> 
> We could give the kmalloc array a different class from created slab
> caches. That should have the desired effect.
> 
> But that seems to be already the case (looking at init_node_lock_keys).
> Non OFF_SLAB caches seem to be getting a different lock class? Why is this
> not working?
> 
> static void init_node_lock_keys(int q)
> {
>         struct cache_sizes *s = malloc_sizes;
> 
>         if (g_cpucache_up != FULL)
>                 return;
> 
>         for (s = malloc_sizes; s->cs_size != ULONG_MAX; s++) {
>                 struct kmem_list3 *l3;
> 
>                 l3 = s->cs_cachep->nodelists[q];
>                 if (!l3 || OFF_SLAB(s->cs_cachep))
>                         continue;
> 
>                 slab_set_lock_classes(s->cs_cachep, &on_slab_l3_key,
>                                 &on_slab_alc_key, q);
>         }
> }

Right, so we recently poked at this to fix some other splats, see:

30765b92ada267c5395fc788623cb15233276f5c
83835b3d9aec8e9f666d8223d8a386814f756266

It could of course be I got confused and broke stuff instead, could
someone who knows slab (I guess that's either Pekka, Christoph or David)
stare at those patches?

Re: lockdep recursive locking detected (rcu_kthread / __cache_free)

[Christoph Lameter]

On Tue, 4 Oct 2011, Peter Zijlstra wrote:

> It could of course be I got confused and broke stuff instead, could
> someone who knows slab (I guess that's either Pekka, Christoph or David)
> stare at those patches?

Why is the loop in init_lock_keys only running over kmalloc caches and not
over all slab caches? It seems that this has to be especially applied to
regular slab caches because those are the ones that mostly have off slab
structures. So modify init_lock_keys to run over all slab caches?

Re: lockdep recursive locking detected (rcu_kthread / __cache_free)

[Peter Zijlstra]

On Tue, 2011-10-04 at 09:50 -0500, Christoph Lameter wrote:
> On Tue, 4 Oct 2011, Peter Zijlstra wrote:
> 
> > It could of course be I got confused and broke stuff instead, could
> > someone who knows slab (I guess that's either Pekka, Christoph or David)
> > stare at those patches?
> 
> Why is the loop in init_lock_keys only running over kmalloc caches and not
> over all slab caches?

A little digging brings us to: 056c62418cc639bf2fe962c6a6ee56054b838bc7
which seems to have introduced that.

>  It seems that this has to be especially applied to
> regular slab caches because those are the ones that mostly have off slab
> structures. So modify init_lock_keys to run over all slab caches?

That sounds about right, worth a try. Also over new caches, the above
reverenced commit removes a hook from kmem_cache_init() which we really
need I suppose.

I'll try and compose a patch if nobody beats me to it, but need to run
an errand first.