[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1292565541.2655.28.camel@edumazet-laptop>
Date: Fri, 17 Dec 2010 06:59:01 +0100
From: Eric Dumazet <eric.dumazet@...il.com>
To: paulmck@...ux.vnet.ibm.com
Cc: Minchan Kim <minchan.kim@...il.com>, gerald.schaefer@...ibm.com,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
Milton Miller <miltonm@....com>, linux-kernel@...r.kernel.org,
linux-mm@...r.kernel.org, linux-ext4@...r.kernel.org,
Ted Ts'o <tytso@....edu>, Arun Bhanu <ab@...nbhanu.com>,
Mel Gorman <mel@....ul.ie>,
Andrew Morton <akpm@...ux-foundation.org>,
Heiko Carstens <heiko.carstens@...ibm.com>,
Martin Schwidefsky <schwidefsky@...ibm.com>
Subject: Re: [BUG?] memory hotplug: include/linux/radix-tree.h:145 invoked
rcu_dereference_check() without protection!
Le jeudi 16 décembre 2010 à 21:47 -0800, Paul E. McKenney a écrit :
> On Fri, Dec 17, 2010 at 09:04:13AM +0900, Minchan Kim wrote:
> > How about this?
> > Maybe Paul have better idea.
> > (It's apparently be word-wrapped.)
> >
> > diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
> > index ab2baa5..135af1e 100644
> > --- a/include/linux/radix-tree.h
> > +++ b/include/linux/radix-tree.h
> > @@ -146,6 +146,20 @@ static inline void *radix_tree_deref_slot(void **pslot)
> > }
> >
> > /**
> > + * radix_tree_deref_slot_nocheck - dereference a slot without RCU check
> > + * @pslot: pointer to slot, returned by radix_tree_lookup_slot
> > + * Returns: item that was stored in that slot with any direct pointer flag
> > + * removed.
> > + *
> > + * This functions works like radix_tree_deref_slot except it doesn't check
> > + * RCU rule. Normally this funcion is used with update-side lock.
> > + * You should use this function very carefully.
> > + */
> > +static inline void *radix_tree_deref_slot_nocheck(void **pslot)
> > +{
> > + return rcu_dereference_protected(*pslot, 1);
>
> I suggest replacing the "1" with lockdep expressions for the locks
> that you say might be held:
>
> return rcu_dereference_check(*pslot,
> lockdep_is_held(&mapping->tree_lock));
>
Yes, but point was also to use rcu_dereference_protected() here, not
rcu_dereference_check().
> This assumes that when you said "and" you meant both lock_page() and
> mapping->tree_lock. Also you need to pass in the mapping, which
> should not be a problem given likely inlining.
>
> If you meant that either mapping->tree_lock or page_lock() might be
> held, I suppose that the page_lock() state could be passed in, but
> perhaps better to take a general lockdep expression.
>
> So, either or both? ;-)
>
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists