Re: Unnecessary barrier in sync_page()?

[Chris Mason <mason@suse.com>]

On Wed, 2004-07-07 at 17:06, Andrea Arcangeli wrote:
> On Wed, Jul 07, 2004 at 04:57:04PM -0400, Chris Mason wrote:
> > I wasn't worried about the locked bit when I added the barrier, my goal
> > was to order things with people that set page->mapping to null.
> 
> page->mapping cannot change from NULL to non-NULL there.
> 
> it can only change from non-NULL to NULL, and there's no way to
> serialize with the truncate without taking the page lock.
> 
> The one extremely important fix you did around the same time, has been
> to "cache" the value of "mapping" in the kernel stack, so that it
> remains the same during the while function (so that it cannot start
> non-NULL an finish NULL). But the smp_mb() itself cannot make a
> difference as far as I can tell.

As Andrew pointed out back then, page->mapping can go to null, but even
if we have a stale copy of page->mapping, the mapping can't be freed. 
So you're right that it should be enough to keep the change to cache the
value of mapping.  

I was hunting the backing dev info bugs back then, and seem to have
talked myself into the barriers while testing...

-chris