2010-06-07 18:23:21

by Miles Lane

[permalink] [raw]
Subject: 2.6.35-rc2-git1 - lib/idr.c:605 invoked rcu_dereference_check() without protection!

[ 2.677955] [ INFO: suspicious rcu_dereference_check() usage. ]
[ 2.679089] ---------------------------------------------------
[ 2.680276] lib/idr.c:605 invoked rcu_dereference_check() without protection!
[ 2.681499]
[ 2.681500] other info that might help us debug this:
[ 2.681501]
[ 2.685509]
[ 2.685510] rcu_scheduler_active = 1, debug_locks = 1
[ 2.688221] 1 lock held by swapper/1:
[ 2.689587] #0: (mtd_table_mutex){+.+...}, at:
[<ffffffff812bea45>] register_mtd_user+0x1a/0x69
[ 2.691096]
[ 2.691098] stack backtrace:
[ 2.694059] Pid: 1, comm: swapper Not tainted 2.6.35-rc2-git1 #8
[ 2.695601] Call Trace:
[ 2.697243] [<ffffffff81064e9c>] lockdep_rcu_dereference+0x9d/0xa5
[ 2.698868] [<ffffffff811b9c86>] idr_get_next+0x60/0x124
[ 2.700556] [<ffffffff812be779>] __mtd_next_device+0x1b/0x1d
[ 2.702238] [<ffffffff812bea7c>] register_mtd_user+0x51/0x69
[ 2.703964] [<ffffffff816cca45>] init_mtdchar+0xb3/0xd3
[ 2.705686] [<ffffffff816cc992>] ? init_mtdchar+0x0/0xd3
[ 2.707470] [<ffffffff810001ef>] do_one_initcall+0x59/0x14e
[ 2.709255] [<ffffffff816a768a>] kernel_init+0x144/0x1ce
[ 2.711082] [<ffffffff81003054>] kernel_thread_helper+0x4/0x10
[ 2.712862] [<ffffffff813ca480>] ? restore_args+0x0/0x30
[ 2.714647] [<ffffffff816a7546>] ? kernel_init+0x0/0x1ce
[ 2.716415] [<ffffffff81003050>] ? kernel_thread_helper+0x0/0x10


2010-06-08 00:13:58

by Paul E. McKenney

[permalink] [raw]
Subject: Re: 2.6.35-rc2-git1 - lib/idr.c:605 invoked rcu_dereference_check() without protection!

On Mon, Jun 07, 2010 at 02:23:17PM -0400, Miles Lane wrote:
> [ 2.677955] [ INFO: suspicious rcu_dereference_check() usage. ]
> [ 2.679089] ---------------------------------------------------
> [ 2.680276] lib/idr.c:605 invoked rcu_dereference_check() without protection!
> [ 2.681499]
> [ 2.681500] other info that might help us debug this:
> [ 2.681501]
> [ 2.685509]
> [ 2.685510] rcu_scheduler_active = 1, debug_locks = 1
> [ 2.688221] 1 lock held by swapper/1:
> [ 2.689587] #0: (mtd_table_mutex){+.+...}, at:
> [<ffffffff812bea45>] register_mtd_user+0x1a/0x69
> [ 2.691096]
> [ 2.691098] stack backtrace:
> [ 2.694059] Pid: 1, comm: swapper Not tainted 2.6.35-rc2-git1 #8
> [ 2.695601] Call Trace:
> [ 2.697243] [<ffffffff81064e9c>] lockdep_rcu_dereference+0x9d/0xa5
> [ 2.698868] [<ffffffff811b9c86>] idr_get_next+0x60/0x124
> [ 2.700556] [<ffffffff812be779>] __mtd_next_device+0x1b/0x1d
> [ 2.702238] [<ffffffff812bea7c>] register_mtd_user+0x51/0x69
> [ 2.703964] [<ffffffff816cca45>] init_mtdchar+0xb3/0xd3
> [ 2.705686] [<ffffffff816cc992>] ? init_mtdchar+0x0/0xd3
> [ 2.707470] [<ffffffff810001ef>] do_one_initcall+0x59/0x14e
> [ 2.709255] [<ffffffff816a768a>] kernel_init+0x144/0x1ce
> [ 2.711082] [<ffffffff81003054>] kernel_thread_helper+0x4/0x10
> [ 2.712862] [<ffffffff813ca480>] ? restore_args+0x0/0x30
> [ 2.714647] [<ffffffff816a7546>] ? kernel_init+0x0/0x1ce
> [ 2.716415] [<ffffffff81003050>] ? kernel_thread_helper+0x0/0x10

This looks like a new one! Does the following patch take care of it?

Thanx, Paul

------------------------------------------------------------------------

commit 2d54a6c31b72c902b09d365e9c66205a5c07e549
Author: Paul E. McKenney <[email protected]>
Date: Mon Jun 7 17:09:45 2010 -0700

idr: fix RCU lockdep splat in idr_get_next()

Convert to rcu_dereference_raw() given that many callers may have many
different locking models.

Located-by: Miles Lane <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>

diff --git a/lib/idr.c b/lib/idr.c
index 2eb1dca..f099f25 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -599,7 +599,7 @@ void *idr_get_next(struct idr *idp, int *nextidp)
/* find first ent */
n = idp->layers * IDR_BITS;
max = 1 << n;
- p = rcu_dereference(idp->top);
+ p = rcu_dereference_raw(idp->top);
if (!p)
return NULL;

@@ -607,7 +607,7 @@ void *idr_get_next(struct idr *idp, int *nextidp)
while (n > 0 && p) {
n -= IDR_BITS;
*paa++ = p;
- p = rcu_dereference(p->ary[(id >> n) & IDR_MASK]);
+ p = rcu_dereference_raw(p->ary[(id >> n) & IDR_MASK]);
}

if (p) {

2010-06-08 04:28:20

by Miles Lane

[permalink] [raw]
Subject: Re: 2.6.35-rc2-git1 - lib/idr.c:605 invoked rcu_dereference_check() without protection!

On Mon, Jun 7, 2010 at 8:12 PM, Paul E. McKenney
<[email protected]> wrote:
> On Mon, Jun 07, 2010 at 02:23:17PM -0400, Miles Lane wrote:
>> [ ? ?2.677955] [ INFO: suspicious rcu_dereference_check() usage. ]
>> [ ? ?2.679089] ---------------------------------------------------
>> [ ? ?2.680276] lib/idr.c:605 invoked rcu_dereference_check() without protection!
>> [ ? ?2.681499]
>> [ ? ?2.681500] other info that might help us debug this:
>> [ ? ?2.681501]
>> [ ? ?2.685509]
>> [ ? ?2.685510] rcu_scheduler_active = 1, debug_locks = 1
>> [ ? ?2.688221] 1 lock held by swapper/1:
>> [ ? ?2.689587] ?#0: ?(mtd_table_mutex){+.+...}, at:
>> [<ffffffff812bea45>] register_mtd_user+0x1a/0x69
>> [ ? ?2.691096]
>> [ ? ?2.691098] stack backtrace:
>> [ ? ?2.694059] Pid: 1, comm: swapper Not tainted 2.6.35-rc2-git1 #8
>> [ ? ?2.695601] Call Trace:
>> [ ? ?2.697243] ?[<ffffffff81064e9c>] lockdep_rcu_dereference+0x9d/0xa5
>> [ ? ?2.698868] ?[<ffffffff811b9c86>] idr_get_next+0x60/0x124
>> [ ? ?2.700556] ?[<ffffffff812be779>] __mtd_next_device+0x1b/0x1d
>> [ ? ?2.702238] ?[<ffffffff812bea7c>] register_mtd_user+0x51/0x69
>> [ ? ?2.703964] ?[<ffffffff816cca45>] init_mtdchar+0xb3/0xd3
>> [ ? ?2.705686] ?[<ffffffff816cc992>] ? init_mtdchar+0x0/0xd3
>> [ ? ?2.707470] ?[<ffffffff810001ef>] do_one_initcall+0x59/0x14e
>> [ ? ?2.709255] ?[<ffffffff816a768a>] kernel_init+0x144/0x1ce
>> [ ? ?2.711082] ?[<ffffffff81003054>] kernel_thread_helper+0x4/0x10
>> [ ? ?2.712862] ?[<ffffffff813ca480>] ? restore_args+0x0/0x30
>> [ ? ?2.714647] ?[<ffffffff816a7546>] ? kernel_init+0x0/0x1ce
>> [ ? ?2.716415] ?[<ffffffff81003050>] ? kernel_thread_helper+0x0/0x10
>
> This looks like a new one! ?Does the following patch take care of it?
>
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit 2d54a6c31b72c902b09d365e9c66205a5c07e549
> Author: Paul E. McKenney <[email protected]>
> Date: ? Mon Jun 7 17:09:45 2010 -0700
>
> ? ?idr: fix RCU lockdep splat in idr_get_next()
>
> ? ?Convert to rcu_dereference_raw() given that many callers may have many
> ? ?different locking models.
>
> ? ?Located-by: Miles Lane <[email protected]>
> ? ?Signed-off-by: Paul E. McKenney <[email protected]>
>
> diff --git a/lib/idr.c b/lib/idr.c
> index 2eb1dca..f099f25 100644
> --- a/lib/idr.c
> +++ b/lib/idr.c
> @@ -599,7 +599,7 @@ void *idr_get_next(struct idr *idp, int *nextidp)
> ? ? ? ?/* find first ent */
> ? ? ? ?n = idp->layers * IDR_BITS;
> ? ? ? ?max = 1 << n;
> - ? ? ? p = rcu_dereference(idp->top);
> + ? ? ? p = rcu_dereference_raw(idp->top);
> ? ? ? ?if (!p)
> ? ? ? ? ? ? ? ?return NULL;
>
> @@ -607,7 +607,7 @@ void *idr_get_next(struct idr *idp, int *nextidp)
> ? ? ? ? ? ? ? ?while (n > 0 && p) {
> ? ? ? ? ? ? ? ? ? ? ? ?n -= IDR_BITS;
> ? ? ? ? ? ? ? ? ? ? ? ?*paa++ = p;
> - ? ? ? ? ? ? ? ? ? ? ? p = rcu_dereference(p->ary[(id >> n) & IDR_MASK]);
> + ? ? ? ? ? ? ? ? ? ? ? p = rcu_dereference_raw(p->ary[(id >> n) & IDR_MASK]);
> ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ?if (p) {
>

Tested. Looks good!

2010-06-08 16:25:37

by Paul E. McKenney

[permalink] [raw]
Subject: Re: 2.6.35-rc2-git1 - lib/idr.c:605 invoked rcu_dereference_check() without protection!

On Tue, Jun 08, 2010 at 12:28:15AM -0400, Miles Lane wrote:
> On Mon, Jun 7, 2010 at 8:12 PM, Paul E. McKenney
> <[email protected]> wrote:
> > On Mon, Jun 07, 2010 at 02:23:17PM -0400, Miles Lane wrote:
> >> [ ? ?2.677955] [ INFO: suspicious rcu_dereference_check() usage. ]
> >> [ ? ?2.679089] ---------------------------------------------------
> >> [ ? ?2.680276] lib/idr.c:605 invoked rcu_dereference_check() without protection!
> >> [ ? ?2.681499]
> >> [ ? ?2.681500] other info that might help us debug this:
> >> [ ? ?2.681501]
> >> [ ? ?2.685509]
> >> [ ? ?2.685510] rcu_scheduler_active = 1, debug_locks = 1
> >> [ ? ?2.688221] 1 lock held by swapper/1:
> >> [ ? ?2.689587] ?#0: ?(mtd_table_mutex){+.+...}, at:
> >> [<ffffffff812bea45>] register_mtd_user+0x1a/0x69
> >> [ ? ?2.691096]
> >> [ ? ?2.691098] stack backtrace:
> >> [ ? ?2.694059] Pid: 1, comm: swapper Not tainted 2.6.35-rc2-git1 #8
> >> [ ? ?2.695601] Call Trace:
> >> [ ? ?2.697243] ?[<ffffffff81064e9c>] lockdep_rcu_dereference+0x9d/0xa5
> >> [ ? ?2.698868] ?[<ffffffff811b9c86>] idr_get_next+0x60/0x124
> >> [ ? ?2.700556] ?[<ffffffff812be779>] __mtd_next_device+0x1b/0x1d
> >> [ ? ?2.702238] ?[<ffffffff812bea7c>] register_mtd_user+0x51/0x69
> >> [ ? ?2.703964] ?[<ffffffff816cca45>] init_mtdchar+0xb3/0xd3
> >> [ ? ?2.705686] ?[<ffffffff816cc992>] ? init_mtdchar+0x0/0xd3
> >> [ ? ?2.707470] ?[<ffffffff810001ef>] do_one_initcall+0x59/0x14e
> >> [ ? ?2.709255] ?[<ffffffff816a768a>] kernel_init+0x144/0x1ce
> >> [ ? ?2.711082] ?[<ffffffff81003054>] kernel_thread_helper+0x4/0x10
> >> [ ? ?2.712862] ?[<ffffffff813ca480>] ? restore_args+0x0/0x30
> >> [ ? ?2.714647] ?[<ffffffff816a7546>] ? kernel_init+0x0/0x1ce
> >> [ ? ?2.716415] ?[<ffffffff81003050>] ? kernel_thread_helper+0x0/0x10
> >
> > This looks like a new one! ?Does the following patch take care of it?
> >
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> > commit 2d54a6c31b72c902b09d365e9c66205a5c07e549
> > Author: Paul E. McKenney <[email protected]>
> > Date: ? Mon Jun 7 17:09:45 2010 -0700
> >
> > ? ?idr: fix RCU lockdep splat in idr_get_next()
> >
> > ? ?Convert to rcu_dereference_raw() given that many callers may have many
> > ? ?different locking models.
> >
> > ? ?Located-by: Miles Lane <[email protected]>
> > ? ?Signed-off-by: Paul E. McKenney <[email protected]>
> >
> > diff --git a/lib/idr.c b/lib/idr.c
> > index 2eb1dca..f099f25 100644
> > --- a/lib/idr.c
> > +++ b/lib/idr.c
> > @@ -599,7 +599,7 @@ void *idr_get_next(struct idr *idp, int *nextidp)
> > ? ? ? ?/* find first ent */
> > ? ? ? ?n = idp->layers * IDR_BITS;
> > ? ? ? ?max = 1 << n;
> > - ? ? ? p = rcu_dereference(idp->top);
> > + ? ? ? p = rcu_dereference_raw(idp->top);
> > ? ? ? ?if (!p)
> > ? ? ? ? ? ? ? ?return NULL;
> >
> > @@ -607,7 +607,7 @@ void *idr_get_next(struct idr *idp, int *nextidp)
> > ? ? ? ? ? ? ? ?while (n > 0 && p) {
> > ? ? ? ? ? ? ? ? ? ? ? ?n -= IDR_BITS;
> > ? ? ? ? ? ? ? ? ? ? ? ?*paa++ = p;
> > - ? ? ? ? ? ? ? ? ? ? ? p = rcu_dereference(p->ary[(id >> n) & IDR_MASK]);
> > + ? ? ? ? ? ? ? ? ? ? ? p = rcu_dereference_raw(p->ary[(id >> n) & IDR_MASK]);
> > ? ? ? ? ? ? ? ?}
> >
> > ? ? ? ? ? ? ? ?if (p) {
> >
>
> Tested. Looks good!

Thank you very much for both locating this one and for testing the fix!
I have added your Tested-by.

Thanx, Paul