Messages in this thread Patch in this message | | | Date | Thu, 12 Aug 1999 21:12:59 -0400 (EDT) | From | Chuck Lever <> | Subject | Re: filemap_nopage() tries to copy to page 0, eek. |
| |
On Thu, 12 Aug 1999, Steve Dodd wrote: > Err, yes, 2.3.12 and 2.3.13 (I think). I'm away from my machine at the > moment so I can't check, but I was using whatever was current when I posted > the message. The simplest option seemed to be to stick a > > if (!new_page) > new_page = page_cache_alloc(); > > just before we attempt to copy to new_page, but I was wondering if we could > overlap the page allocation for new_page and the previous ->readpage()..
here's a cleaner patch i came up with flying back from san jose. it's against 2.3.10, but should apply to 2.3.1[23] without much fiddling. essentially it's the same as what i described yesterday, but there's some additional clean-up and extra comments that go along with the deletion of redundant code. i've been testing a more complicated variant of this for a few weeks, and haven't seen any trouble with it.
Steve, can you try this out and let me know if it helps?
--- linux-2.3.10-ref/mm/filemap.c Sat Jul 10 12:27:06 1999 +++ linux/mm/filemap.c Thu Aug 12 16:03:00 1999 @@ -1302,7 +1302,6 @@ if (!page) goto no_cached_page; -found_page: /* * Ok, found a page in the page cache, now we need to check * that it's up-to-date. First check whether we'll need an @@ -1314,12 +1313,8 @@ goto failure; } - if (!Page_Uptodate(page)) { - lock_page(page); - if (!Page_Uptodate(page)) - goto page_not_uptodate; - UnlockPage(page); - } + if (!Page_Uptodate(page)) + goto page_not_uptodate; success: /* @@ -1358,44 +1353,28 @@ for (i = 1 << page_cluster; i > 0; --i, reada += PAGE_CACHE_SIZE) new_page = try_to_read_ahead(file, reada, new_page); - if (!new_page) - new_page = page_cache_alloc(); - if (!new_page) - goto no_page; - /* - * During getting the above page we might have slept, - * so we need to re-check the situation with the page - * cache.. The page we just got may be useful if we - * can't share, so don't get rid of it here. + * The page we want has now been added to the page cache. + * In the unlikely event that someone removed it in the + * meantime, we'll just come back here and read it again. */ - page = __find_get_page(inode, offset, hash); - if (page) - goto found_page; - - /* - * Now, create a new page-cache page from the page we got - */ - page = page_cache_entry(new_page); - if (add_to_page_cache_unique(page, inode, offset, hash)) - goto retry_find; - - /* - * Now it's ours and locked, we can do initial IO to it: - */ - new_page = 0; + goto retry_find; page_not_uptodate: + lock_page(page); + if (Page_Uptodate(page)) { + UnlockPage(page); + goto success; + } + error = inode->i_op->readpage(file, page); if (!error) { wait_on_page(page); - if (PageError(page)) - goto page_read_error; - goto success; + if (Page_Uptodate(page)) + goto success; } -page_read_error: /* * Umm, take care of errors if the page isn't up-to-date. * Try to re-read it _once_. We do this synchronously, - Chuck Lever -- corporate: <chuckl@netscape.com> personal: <chucklever@netscape.net> or <cel@monkey.org>
The Linux Scalability project: http://www.citi.umich.edu/projects/linux-scalability/
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |