lkml.org 
[lkml]   [2010]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [BUG?] [Ext4] INFO: suspicious rcu_dereference_check() usage
Date
On 2010-11-22 at around 0:38:49, Minchan Kim wrote:
> On Mon, Nov 22, 2010 at 2:37 AM, Ted Ts'o <tytso@mit.edu> wrote:
> > On Mon, Nov 22, 2010 at 12:39:49AM +0900, Minchan Kim wrote:
> > >
> > > I think it's no problem.
> > >
> > > That's because migration always holds lock_page on the file page.
> > > So the page couldn't remove from radix.
> >
> > It may be "ok" in that it won't cause a race, but it still leaves an
> > unsightly warning if LOCKDEP is enabled, and LOCKDEP warnings will
> > cause /proc_lock_stat to be disabled. So I think it still needs to be
> > fixed by adding rcu_read_lock()/rcu_read_unlock() to
> > migrate_page_move_mapping().
> >
> > - Ted
> >
>
> Yes. if it is really "ok" about race, we will add rcu_read_lock with
> below comment to prevent false positive.
> "suppress RCU lockdep false positives".
> But I am not sure it's good although rcu_read_lock is little cost.
> Whenever we find false positive, should we add rcu_read_lock to
> suppress although it's no problem in real product?
> Couldn't we provide following function? (or we might have already it
> but I missed it. )
>
> /*
> * Suppress RCU lockdep false positive.
> */
> #ifdef CONFIG_PROVE_RCU
> #define rcu_read_lock_suppress rcu_read_lock
> #else
> #define rcu_read_lock_suppress
> #endif

No, you don't need anything like this, as rcu_dereference_check already
takes a test for alternate locking.

However, looking more closely at the code, it appears this is the
"the tree is write locked" case as described in radix-tree.h

Looking at rcupdate.h, perhaps we need a version of radix_tree_deref_slot
that uses rcu_dereference_protected?

Copying Paul McKenney for rcu ...

milton


\
 
 \ /
  Last update: 2010-11-22 04:33    [W:0.083 / U:0.480 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site