[Top] [All Lists]

Re: [patch] mm: fix lockless pagecache reordering bug (was Re: BUG: soft

To: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Subject: Re: [patch] mm: fix lockless pagecache reordering bug (was Re: BUG: soft lockup - is this XFS problem?)
From: "Paul E. McKenney" <paulmck@xxxxxxxxxxxxxxxxxx>
Date: Mon, 5 Jan 2009 12:12:58 -0800
Cc: Nick Piggin <npiggin@xxxxxxx>, Peter Klotz <peter.klotz@xxxxxx>, stable@xxxxxxxxxx, Linux Memory Management List <linux-mm@xxxxxxxxx>, Christoph Hellwig <hch@xxxxxxxxxxxxx>, Roman Kononov <kernel@xxxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
In-reply-to: <alpine.LFD.2.00.0901051027011.3057@xxxxxxxxxxxxxxxxxxxxx>
References: <20081230042333.GC27679@xxxxxxxxxxxxx> <20090103214443.GA6612@xxxxxxxxxxxxx> <20090105014821.GA367@xxxxxxxxxxxxx> <20090105041959.GC367@xxxxxxxxxxxxx> <20090105064838.GA5209@xxxxxxxxxxxxx> <49623384.2070801@xxxxxx> <20090105164135.GC32675@xxxxxxxxxxxxx> <alpine.LFD.2.00.0901050859430.3057@xxxxxxxxxxxxxxxxxxxxx> <20090105180008.GE32675@xxxxxxxxxxxxx> <alpine.LFD.2.00.0901051027011.3057@xxxxxxxxxxxxxxxxxxxxx>
Reply-to: paulmck@xxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.15+20070412 (2007-04-11)
On Mon, Jan 05, 2009 at 10:44:27AM -0800, Linus Torvalds wrote:
> On Mon, 5 Jan 2009, Nick Piggin wrote:
> > On Mon, Jan 05, 2009 at 09:30:55AM -0800, Linus Torvalds wrote:
> > Putting an rcu_dereference there might work, but I think it misses a 
> > subtlety of this code.
> No, _you_ miss the subtlety of something that can change under you.
> Look at radix_tree_deref_slot(), and realize that without the 
> rcu_dereference(), the compiler would actually be allowed to think that it 
> can re-load anything from *pslot several times. So without my one-liner 
> patch, the compiler can actually do this:
>       register = load_from_memory(pslot)
>       if (radix_tree_is_indirect_ptr(register))
>               goto fail:
>       return load_from_memory(pslot);
>    fail:
>       return RADIX_TREE_RETRY;

My guess is that Nick believes that the value in *pslot cannot change
in such as way as to cause radix_tree_is_indirect_ptr()'s return value
to change within a given RCU grace period, and that Linus disagrees.

Whatever the answer, I would argue for -at- -least- a comment explaining
why it is safe.  I am not seeing the objection to rcu_dereference(), but
I must confess that it has been awhile since I have looked closely at
the radix_tree code.  :-/

                                                        Thanx, Paul

<Prev in Thread] Current Thread [Next in Thread>