Commit cff6ab5e authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[PATCH] pagecache locking bugfix

The bug which Anton found.  On the
find_or_create_page->__find_lock_page path we're performing
a read_unlock of an rwlock which is held for writing.

The patch converts that to using a write_lock throughout.

Which penalises find_lock_page() a bit.  If it shows up
on profiles then we can clone __find_lock_page() and
use read_lock()s, but for now I'd opt for saving the
cache footprint.
parent 0d9bef55
...@@ -797,9 +797,9 @@ struct page *find_trylock_page(struct address_space *mapping, unsigned long offs ...@@ -797,9 +797,9 @@ struct page *find_trylock_page(struct address_space *mapping, unsigned long offs
} }
/* /*
* Must be called with the pagecache lock held, * Must be called with the mapping lock held for writing.
* will return with it held (but it may be dropped * Will return with it held for writing, but it may be dropped
* during blocking operations.. * while locking the page.
*/ */
static struct page *__find_lock_page(struct address_space *mapping, static struct page *__find_lock_page(struct address_space *mapping,
unsigned long offset) unsigned long offset)
...@@ -815,11 +815,11 @@ static struct page *__find_lock_page(struct address_space *mapping, ...@@ -815,11 +815,11 @@ static struct page *__find_lock_page(struct address_space *mapping,
if (page) { if (page) {
page_cache_get(page); page_cache_get(page);
if (TryLockPage(page)) { if (TryLockPage(page)) {
read_unlock(&mapping->page_lock); write_unlock(&mapping->page_lock);
lock_page(page); lock_page(page);
read_lock(&mapping->page_lock); write_lock(&mapping->page_lock);
/* Has the page been re-allocated while we slept? */ /* Has the page been truncated while we slept? */
if (page->mapping != mapping || page->index != offset) { if (page->mapping != mapping || page->index != offset) {
UnlockPage(page); UnlockPage(page);
page_cache_release(page); page_cache_release(page);
...@@ -830,25 +830,53 @@ static struct page *__find_lock_page(struct address_space *mapping, ...@@ -830,25 +830,53 @@ static struct page *__find_lock_page(struct address_space *mapping,
return page; return page;
} }
/**
* find_lock_page - locate, pin and lock a pagecache page
*
* @mapping - the address_space to search
* @offset - the page index
*
* Locates the desired pagecache page, locks it, increments its reference
* count and returns its address.
*
* Returns zero if the page was not present. find_lock_page() may sleep.
*/
/* /*
* Same as the above, but lock the page too, verifying that * The write_lock is unfortunate, but __find_lock_page() requires that on
* it's still valid once we own it. * behalf of find_or_create_page(). We could just clone __find_lock_page() -
* one for find_lock_page(), one for find_or_create_page()...
*/ */
struct page * find_lock_page(struct address_space *mapping, unsigned long offset) struct page *find_lock_page(struct address_space *mapping,
unsigned long offset)
{ {
struct page *page; struct page *page;
read_lock(&mapping->page_lock); write_lock(&mapping->page_lock);
page = __find_lock_page(mapping, offset); page = __find_lock_page(mapping, offset);
read_unlock(&mapping->page_lock); write_unlock(&mapping->page_lock);
return page; return page;
} }
/* /**
* Same as above, but create the page if required.. * find_or_create_page - locate or add a pagecache page
*
* @mapping - the page's address_space
* @index - the page's index into the mapping
* @gfp_mask - page allocation mode
*
* Locates a page in the pagecache. If the page is not present, a new page
* is allocated using @gfp_mask and is added to the pagecache and to the VM's
* LRU list. The returned page is locked and has its reference count
* incremented.
*
* find_or_create_page() may sleep, even if @gfp_flags specifies an atomic
* allocation!
*
* find_or_create_page() returns the desired page's address, or zero on
* memory exhaustion.
*/ */
struct page * find_or_create_page(struct address_space *mapping, struct page *find_or_create_page(struct address_space *mapping,
unsigned long index, unsigned int gfp_mask) unsigned long index, unsigned int gfp_mask)
{ {
struct page *page; struct page *page;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment