Re: [PATCH] AFS: Implement shared-writable mmap [try #2]

[David Howells <dhowells@redhat.com>]

Nick Piggin <nickpiggin@yahoo.com.au> wrote:

> > 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?

Because invalidate_inode_pages() forcibly removes the dirty flag from each page
in the inode and then calls invalidatepage() - and thus they don't get written
back, but some of those pages may contain writes from other processes.  The
whole inode isn't owned by one user at a time.

I hadn't considered invalidate_inode_pages_range(), but that suffers from the
deadlock problem.

> > 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?

 (1) prepare_write() is called with the target page locked and does not release
     the lock.  The truncation routines lock the page prior to invalidating it.
     Any truncation routine that skips locked pages is of no use.

 (2) Consider a run of pages that make up a single write by one user.  Two
     other writes from other users may be attempting to overwrite that run at
     the same time.  Each of them would need to invalidate the other's locked
     page(s).

Furthermore, the caller of prepare_write() probably won't take kindly to the
page it's dealing with being evicted from the pagecache.

> 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?

How do you do a write-through cache for shared-writable mmap?

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

There might be, yes.  I guess I should ask the VM to nuke all PTEs to each of
these pages too.

David