From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [PATCH] AFS: Implement shared-writable mmap [try #2] Date: Thu, 17 May 2007 02:32:09 +1000 Message-ID: <464B3209.4010003@yahoo.com.au> References: <464B07EC.4050308@yahoo.com.au> <464AF3F3.30204@yahoo.com.au> <20070516100225.18685.51699.stgit@warthog.cambridge.redhat.com> <17173.1179321391@redhat.com> <19714.1179331928@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Cc: akpm@osdl.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org To: David Howells Return-path: Received: from smtp105.mail.mud.yahoo.com ([209.191.85.215]:40831 "HELO smtp105.mail.mud.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1756274AbXEPQcP (ORCPT ); Wed, 16 May 2007 12:32:15 -0400 In-Reply-To: <19714.1179331928@redhat.com> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org David Howells wrote: > Nick Piggin 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.