Re: [PATCH] Scalable page cache

Linus Torvalds (torvalds@transmeta.com)
Mon, 26 Nov 2001 12:37:47 -0800


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).

So you should just remove all the kmap/kunmap stuff in
prepare/commit_write instead of adding more of them to handle the
brokenness.

Linus
-
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/