Messages in this thread |  | | Date | Mon, 26 Nov 2001 13:02:42 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] Scalable page cache |
| |
Linus Torvalds wrote: > > In article <3C02A00B.B9948342@zip.com.au> you write: > > > >Here's the current rolled-up ext3 diff which I've been using. > >It needs to be resynced with pestiferous ext3 CVS, changelogged > >and pushed out this week. > > > >--- linux-2.4.15-pre9/fs/ext3/inode.c Thu Nov 22 10:56:33 2001 > >+++ linux-akpm/fs/ext3/inode.c Thu Nov 22 11:07:03 2001 > >@@ -1026,9 +1026,20 @@ static int ext3_prepare_write(struct fil > > if (ret != 0) > > goto prepare_write_failed; > > > >- if (ext3_should_journal_data(inode)) > >+ if (ext3_should_journal_data(inode)) { > > ret = walk_page_buffers(handle, page->buffers, > > from, to, NULL, do_journal_get_write_access); > >+ if (ret) { > >+ /* > >+ * We're going to fail this prepare_write(), > >+ * so commit_write() will not be called. > >+ * We need to undo block_prepare_write()'s kmap(). > >+ * AKPM: Do we need to clear PageUptodate? I don't > >+ * think so. > >+ */ > >+ kunmap(page); > >+ } > >+ } > > This is wrong. > > The generic VM layer does the kmap/kunmap itself these days (it really > always should have - it needs the page mapped itself for the memcpy > anyway, and depending on the low-level FS to do kmap/kunmap was an ugly > layering violation).
Actually the comment is a bit misleading.
We've called block_prepare_write(), which has done the kmap. But even though block_prepare_write() returned success, this call to the filesystem's ->prepare_write() is about to fail.
So the caller of ->prepare_write() isn't going to run ->commit_write(), and we have to undo the kmap here.
> So you should just remove all the kmap/kunmap stuff in > prepare/commit_write instead of adding more of them to handle the > brokenness.
There have been a number of mistakes made over this particular kmap() operation. NFS client had it wrong for a while. I think sct had some proposal for making it more robust.
- - 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/
|  |