Re: CONFIRMED bug in do_generic_file_read

[Al Viro <viro@parcelfarce.linux.theplanet.co.uk>]

On Fri, May 13, 2005 at 06:42:50AM -0700, James Washer wrote:
> Al, relax.. as I said, I don't know much about page cache code. 
> 
> So, let me ask a question, if I can, with out upsetting you further.
> 
> You say the analysis is, ah, incorrect.
> 
> Can you help me understand what a readpage routine SHOULD do with a page 
> when it finds it cannot "arrange" a successful read? Is simply returning 
> an error incorrect behaviour? If so, what should the readpage do?

It is a perfectly acceptable behaviour.  And it works just fine - e.g.
nfs_readpage() does that in quite a few cases.

What you are missing is the fact that page_cache_release() frees the
page only when it drops the final reference.  And pages are pinned
down while they are in page cache.

If you see page_cache_release() right after ->readpage() triggering that
check, you've got out of ->readpage() with
	* only one reference to page remaining
	* one reference to that page acquired earlier in do_generic_file_read()
and not dropped until now.
	* one reference to that page acquired back when it had been put
in page cache.  Matching page_cache_release() would be done when page
is removed from page cache, but places that do it would remove the page
from cache first.  Which would set ->mapping to NULL.

Conclusion: something had done an unbalanced page_cache_release().  That
happened after the moment when do_generic_file_read() had found the page
and pinned it down and before the end of ->readpage().  Most likely -
->readpage() itself or something called by it.
the page, but