Re: Problems with mmap consistency

[Andrea Arcangeli <andrea@suse.de>]

On Fri, Feb 24, 2006 at 05:17:52PM -0800, Andrew Morton wrote:
> Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> >
> > On Sat, 2006-02-25 at 01:33 +0100, Andrea Arcangeli wrote:
> > 
> > > I got positive confirmation that my patch that makes
> > > invalidate_inode_pages2 non-destructive fixed the problem. At the top of
> > > this thread you can find the testcase used to reproduce the race posted
> > > by Olaf. I'm unsure if Neil's patch is needed, but it certainly could
> > > co-exist. I feel his patch should not execute a writepage inside
> > > try_to_release_page, it's not needed anymore with my fix in place.
> > 
> > Yes it still is needed. The page doesn't need to have the dirty bit set
> > in order to actually need writing out (sort of the equivalent of having
> > buffers in the blocks case).
> 
> eh?  I don't know what's being talked about here, but that doesn't sound
> right.  Taking the block-backed protocol as an example, if a page isn't
> dirty it shouldn't be written out.  What _is_ legal is a page which is
> dirty but doesn't need writing out.  You've gone and invented the converse,
> illegal case here.

Agreed, this is what I was trying to say too in the previous emails in
answer to Neil (and it was confirmed by your last email too). It doesn't
sound sane that a page has dirty-lowlevel data but PG_dirty is not set.
try_to_release_page isn't ->writepage and if PG_dirty isn't set it would
be hiding those pages to pdflush (pdflush wouldn't see the dirty bit and
it wouldn't writeback periodically).

try_to_release_page can return failure without doing writeback if it
finds dirty data in the lowlevel caches (but even that isn't strictly
necessary as long as the PG_dirty bit is set correctly).

Now the fact my patch fixed the problem, makes me quite optimistic nfs
is currently doing the right thing in setting PG_dirty, but perhaps the
testcase isn't stressing all races...

> > My patch just ensures that try_to_release_page() actually flushes those
> > buffers and waits for completion.
> 
> <what patch?>

The one that Neil posted, it was the second URL (the first url is my
proposed patch):

http://sourceforge.net/mailarchive/message.php?msg_id=14906347
http://sourceforge.net/mailarchive/message.php?msg_id=14906867


-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs