Re: [BUG?] memory hotplug: include/linux/radix-tree.h:145 invokedrcu_dereference_check() without protection!

[Eric Dumazet]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/