Re: [patch] fs: improved handling of page and buffer IO errors

[Nick Piggin <npiggin@suse.de>]

On Thu, Oct 23, 2008 at 11:52:11AM +0100, steve@chygwyn.com wrote:
> Hi,
> 
> On Thu, Oct 23, 2008 at 12:21:00PM +0200, Nick Piggin wrote:
> > On Thu, Oct 23, 2008 at 10:59:49AM +0100, steve@chygwyn.com wrote:
> > > > Btw, at least for the readpage case, a return of AOP_TRUNCATED_PAGE should
> > > > be checked for, which would indicate (along with !PageUptodate()) whether we
> > > > need to retry the read. page_mkwrite though, as you point out, is a
> > > > different story.
> > > > 	--Mark
> > > >
> > > Yes, and although I probably didn't make it clear I was thinking
> > > specifically of the page fault path there where both readpage and
> > > page_mkwrite hang out.
> > > 
> > > Also, I've looked through all the current GFS2 code and it seems to
> > > be correct in relation to Miklos' point on PageUptodate() vs
> > > page->mapping == NULL so I don't think any changes are required there,
> > > but obviously that needs to be taken into account in filemap_fault wrt
> > > to retrying in the lock demotion case. In other words we should be
> > > testing for page->mapping == NULL rather than !PageUptodate() in that
> > > case,
> > 
> > PageUptodate is OK for the filemap_fault check AFAIKS, because it does
> > a find_lock_page and runs the check under lock (so it can't be truncated
> > or invalidated), in order to prevent fault vs truncate / invalidate races.
> > 
> 
> Ah yes, I see now. Sorry, my fault (no pun intended!). Its the test after
> readpage that I was thinking of, for which I'd previously posted a
> patch under the subject "Potential fix to filemap_fault()" and you'd
> reponded with a more comprehensive patch, back in July,

Yes, this is roughly the same patch, although probably that part of it
was valid when you posted it as we probably still cleared PG_uptodate
on invalidate at that time...

I still think there are some improvements we can make though, so I
have picked up the patch again and started trying to do more testing
of it