On Mon, 30 Oct 2000, Alexander Viro wrote:
>
> Unfortunately, it doesn't fix the thing. ->sync_page() is called when we
> do not own the page lock and nfs_sync_page() uses page->mapping. Yes, we
> check it before calling the bloody thing, but we don't own the lock.
Good catch.
> Problem only for NFS, but I'm not sure what to do about it - the whole
> point of ->sync_page() seems to be (if I understood Trond's intentions
> right) in forcing the ->readpage() in progress.
How about just changing ->sync_page() semantics to own the page lock? That
sound slike the right thing anyway, no?
> Another place you've missed is in read_cache_page(). That one is easy - we've
> just locked the page and we should just repeat the whole thing if it's out
> of cache.
I didn't actually miss it, I just looked at the users and decided that it
looks like they should never have this issue. But I might have missed
something. As far as I can tell, "read_cache_page()" is only used for
meta-data like things that cannot be truncated.
But you're right, we should do it for consistency.
> One more is in filemap_swapout() - dunno, I just shifted the check to
> filemap_write_page().
I'd really like to do these in the thing that locks the page, and make the
rule be that the page locker needs to do the work. That's why I'd prefer
to let the test be in the _caller_ of filemap_write_page(), as that's the
point where we got the lock.
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Tue Oct 31 2000 - 21:00:27 EST