Re: [PATCH 2/4] AFS: Add a function to excise a rejected write from the pagecache

[Trond Myklebust <trond.myklebust@fys.uio.no>]

On Fri, 2007-05-25 at 00:18 +0100, David Howells wrote:
> Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> 
> > No. If the write fails, then NFS will mark the mapping as invalid and
> > attempt to call invalidate_inode_pages2() at the earliest possible
> > moment.
> 
> Will it erase *all* unwritten writes?  Or is that what launder_page() is for?

Yes. launder_page() will flush out any writes that may have raced with
the call to invalidate_inode_pages2() (you're supposed to attempt to
flush them out before you call the latter).

> How do you deal with pages that were in the process of being written out when
> that particular write was rejected?  Do you just summarily clear PG_writeback
> and hope no-one else looks at the page until invalidate_inode_pages2() gets
> around to excising it?  Or do you have a better way?

All I do is to protect new calls to read() and write() with a call to
check if the page cache needs invalidating. As I said earlier, you
cannot avoid the race. At best you can reduce the window of opportunity,
and so I'd argue that if you can't do that cheaply, then it isn't worth
it.

> > I'm adding in a patch to defer marking the page as uptodate until the
> > write is successful in cases where NFS is writing a pristine page.
> 
> That sounds reasonable, though it doesn't help in the case I'm looking at.  Do
> you also munge i_size if the write fails?

I just mark the inode as needing revalidation so that we update the size
on the next read()/write()/getattr(). That won't stop any existing
append writes from punching ugly holes into the file, but trying to
recover from that sort of thing would be _really_ painful!

> > As for pages that are already marked as uptodate, well you already have
> > a race: you have deferred the page write, and so other processes may
> > already have read the rejected data before you tried to write it out.
> 
> Yeah, I know, and that's very difficult to deal with without some formal
> transaction rollback mechanism.  I think that the best I can do is to discard
> the dodgy data that I've got lurking in the pagecache, but I still have to
> deal with writes made by other users to that file after the rejected write.
> 
> There isn't a perfect way of dealing with it, given the circumstances.

Agreed.

Trond