Messages in this thread Patch in this message | | | Date | Thu, 19 Aug 1999 22:43:23 +0100 | From | Steve Dodd <> | Subject | Re: [PATCH] 2.3.1[34]: filemap_nopage faults on ->readpage() failure |
| |
On Thu, Aug 19, 1999 at 12:35:22PM -0400, Chuck Lever wrote:
> good spotting... i looked at this a while back with the same suspicion. > locking the page again, as in your patch, will cause a deadlock.
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 ;-)
> 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"?
> if (!error) { > wait_on_page(page); > if (Page_Uptodate(page)) > goto success; > lock_page(page); > ^^^^^^^^^^^^^^^^ > }
Okay, that makes sense. New patch (untested, but 'obviously' correct.. again):
--- 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) > do you have a test case?
Yup, I have a Cheapbytes Debian 2.1R2 CD with a few suitably buggered files. grep'ing them triggers the above problem quite nicely.
-- If smoking is so bad for you, how come it cures kippers?
- 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/
| |