Messages in this thread | | | Date | Fri, 20 Aug 1999 15:32:43 -0400 (EDT) | From | Chuck Lever <> | Subject | Re: [PATCH] 2.3.1[34]: filemap_nopage faults on ->readpage() failure |
| |
On Thu, 19 Aug 1999, Steve Dodd wrote: > Ah yes, if readpage() leaves the page locked? I wasn't sure what other > implementations did (block_read_full_page doesn't ever fail), and I was too > lazy to check ;-)
well, the logic in filemap_nopage expects it to be locked. readpage() won't unlock a page -- it's unlocked when I/O is completed -- unless readpage() completes the I/O synchronously. if an error occurs while a request is being scheduled, then the page is left locked, since no I/O exit will be called to unlock it. that's pretty much what an error return from readpage() should mean, since it automatically triggers a re-drive in filemap_nopage.
> > i think the readpage implementations are careful enough to leave the page > > locked if they return an error, but it might be good to review each. > > nfs/read.c:nfs_readpage() doesn't seem to. Do we fix it, or try to handle this > in filemap_nopage? Ack, smb_readpage is weird, too -- it seems to expect the > page /not/ to be locked. Am I right in thinking that smbfs hasn't been > sanitised since the "Great Overhaul"?
i think all that code deserves review.
> --- filemap.c.orig Thu Aug 19 08:37:26 1999 > +++ filemap.c Thu Aug 19 22:41:39 1999 > @@ -1378,6 +1378,7 @@ > wait_on_page(page); > if (Page_Uptodate(page)) > goto success; > + lock_page(page); > } > > /* > @@ -1386,8 +1387,11 @@ > * because there really aren't any performance issues here > * and we need to check for errors. > */ > - if (!PageLocked(page)) > - PAGE_BUG(page); > + if (Page_Uptodate(page)) { > + UnlockPage(page); > + goto success; > + } > + > ClearPageError(page); > error = inode->i_op->readpage(file, page); > if (error)
i'm not sure that this Page_Uptodate check is needed...
- 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/
| |