Re: CONFIRMED bug in do_generic_file_read

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

On Fri, May 13, 2005 at 10:55:24AM -0700, James Washer wrote:
> In do_generic_file_read(), when the page is not found, it is created, and 

.. with refcount 1

> __add_to_page_cache() is called, which will in turn call page_cache_get() 
> which gives us a page->count of 1, no?

That would be 2.

> Now, we "goto reapage",  which calls a_op->readpage. If this readpage 
> simply returns an error, without any other actions, we drop  down to the 
> page_cache_release(). Finding the page->count==0, we'll proceed to call 
> __free_page(), which calls __free_pages() which decrements and tests 
> page->count via put_page_testzero(), returning true as page->count is now 
> zero...  and were off to __free_pages_ok() and our panic...
> 
> What did I miss? Forgive me being dense, if I'm missing something here. 
> And thanks again for you help understanding this.

	Think for a minute - we allocate a refcounted object.  That means
getting (the only) reference to it.  That means refcount equal to 1.  It's
a fairly common idiom - not just pages are handled that way.

	What happens is:
* We created a page.  We have a reference to it.
* We put it into cache.  Now there are two ways to reach it - our reference
and global search structure.
* We do ->readpage() - we know that we have a reference, so it's not going
away
* We drop our reference.  That does not affect the presence in cache - it's
not our responsibility anymore.  We are done with this page; if somebody
decides to kick it out of cache, it will be their resposibility to drop the
reference created when putting into cache.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/