Messages in this thread | | | Date | Thu, 17 May 2007 02:32:09 +1000 | From | Nick Piggin <> | Subject | Re: [PATCH] AFS: Implement shared-writable mmap [try #2] |
| |
David Howells wrote: > Nick Piggin <nickpiggin@yahoo.com.au> wrote: > > >>In general (modulo bugs and crazy filesystems), you're not allowed to have >>!uptodate pages mapped into user addresses because that implies the user >>would be allowed to see garbage. > > > Ths situation I have to deal with is a tricky one. Consider: > > (1) User A modifies a page with his key. This change gets made in the > pagecache, but is not written back immediately. > > (2) User B then wants to modify the same page, but with a different key. > This means that afs_prepare_write() has to flush A's writes back to the > server before B is permitted to write. > > (3) The flush fails because A is no longer permitted to write to that file. > This means that the change in the page cache is now stale. We can't just > write it back as B because B didn't make the change. > > What I've made afs_prepare_write() do in this situation is to nuke A's entire > write. We can't write any of it back. I can't call invalidate_inode_pages() > or similar because that might incorrectly kill one of B's writes (or someone > else's writes); besides, the on-server file hasn't changed.
Why would that kill anyone's writes?
> To nuke A's write, each page that makes up that write is marked non-uptodate > and then reloaded. Whilst I might wish to call invalidate_inode_pages_range(), > I can't as it can/would deadlock if called from prepare_write() in two > different ways.
Which ways? Are you talking about prepare_write being called from page_mkwrite, or anywhere?
More generally it sounds like a nasty thing to have a writeback cache if it can become incoherent (due to dirty pages that subsequently cannot be written back) without notification. Have you tried doing a write-through one?
You may be clearing PG_uptodate, but isn't there still an underlying problem that you can have mappings to the page at that point? If that isn't a problem for you, then I don't know why you would have to clear PG_uptodate at all.
>>>>Minor issue: you can just check for `if (!page->mapping)` for truncation, >>>>which is the usual signal to tell the reader you're checking for truncate. >>> >>> >>>That's inconsistent with other core code, truncate_complete_page() for >>>example. >> >>Your filesystem internally moves pages between mappings like tmpfs? > > > You misunderstand me. truncate_complete_page() uses this: > > if (page->mapping != mapping) > > not this: > > if (!page->mapping) > > I think that both cases should work in page_mkwrite(). But !page->mapping does > not appear to be the "usual signal" from what I've seen.
truncate_complete_page does that because it has to handle the case where the mapping changes from one thing to something else that is non-NULL, which tmpfs does.
This is not the case for most code in fs.
-- SUSE Labs, Novell Inc. - 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/
| |