Re: [PATCH 1 of 2] block_page_mkwrite() Implementation V2

From: Nick Piggin
Date: Wed May 16 2007 - 09:42:05 EST


David Howells wrote:
Nick Piggin <nickpiggin@xxxxxxxxxxxx> wrote:


Dave is using prepare_write here to ensure blocks are allocated in the
given range. The filesystem's ->nopage function must ensure it is uptodate
before allowing it to be mapped.


Which is fine... assuming it's called. For blockdev-based filesystems, this
is probably true. But I'm not sure you can guarantee it.

I've seen Ext3, for example, unlocking a page that isn't yet uptodate.
nopage() won't get called on it again, but prepare_write() might. I don't
know why this happens, but it's something I've fallen over in doing
CacheFiles. When reading, readpage() is just called on it again and again
until it is up to date. When writing, prepare_write() is called correctly.

There are bugs in the core VM and block filesystem code where !uptodate pages
are left in pagetables. Some of these are fixed in -mm.

But they aren't a good reason to invent completely different ways to do things.


Consider that the code currently works OK today _without_ page_mkwrite.
page_mkwrite is being added to do block allocation / reservation.


Which doesn't prove anything. All it means is that PG_uptodate being unset is
handled elsewhere.

It means that Dave's page_mkwrite function will do the block allocation
and everything else continues as it is. Your suggested change to pass in
offset == to is just completely wrong for this.

PG_uptodate being unset should be done via pagecache invalidation or truncation
APIs, which (sometimes... modulo bugs) tear down pagetables first.

--
SUSE Labs, Novell Inc.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/