Re: stable page writes: wait_on_page_writeback and packet signing

[Chris Mason <chris.mason-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]

Excerpts from Jeff Layton's message of 2011-03-10 08:47:24 -0500:
> > > > Does cifs take i_mutex before writepage? The disk based filesystems
> > > > don't.  So, i_mutex protects file_write from other procs jumping into
> > > > file_write, but it doesn't protect writeback from file_write jumping in
> > > > and changing the pages while they are being sent to storage (or over the
> > > > wire).
> > > > 
> > > > Basically the model needs to be:
> > > > 
> > > > file_write:
> > > >     lock the page
> > > >     wait on page writeback
> > > > 
> > > >     < new writeback cannot start because of the page lock >
> > > >     copy_from_user
> > > >     unlock the page
> > > > 
> > > > We also use page_mkwrite to get notified when userland wants to change
> > > > some page it has given to mmap.  That needs to wait on page writeback as
> > > > well.
> > > > 
> > > 
> > > No, cifs doesn't take the i_mutex in writepage, but the page is locked.
> > > cifs_write_begin calls grab_cache_page_write_begin, which returns a
> > > locked page and it's not unlocked until cifs_write_end.
> > 
> > Ah ok, so you've got the page locked the whole time it is being sent
> > over the wire?  The disk based filesystems split it and drop the page
> > lock once the page is set writeback, which is why we need the extra
> > waits.
> > 
> > So in your case you should just need a page_mkwrite that locks the page.
> > 
> 
> Right, or do a wait_on_page_writeback. I think that may have a little
> less overhead since we won't need to unlock it and it may mean less
> serialization if there are other contenders for the page lock.
> 

I think you'll need the page lock too, otherwise you aren't protected
against new IO starting.  page_mkwrite really works together with 
clear_page_dirty_for_io(), and I don't think you get proper
synchronization without the page lock.

You also need the page lock to make sure the page really is still in
your mapping and that truncate won't race in and take the page away.

Basically, if you add a wait_on_page_writeback() to block_page_mkwrite,
you should have what you need, +/- the prepare_write/commit_write calls,
which we use to fill any holes for the pages that are about to change.

-chris