[linux-cifs-client] Re: [PATCH] cifs: prevent cifs_writepages() from skipping unwritten pages

[Jeff Layton]

On Mon, 17 Nov 2008 09:26:57 -0600
Dave Kleikamp <shaggy at linux.vnet.ibm.com> wrote:

> On Mon, 2008-11-17 at 10:19 -0500, Jeff Layton wrote:
> > On Mon, 17 Nov 2008 07:02:47 -0600
> > Dave Kleikamp <shaggy at linux.vnet.ibm.com> wrote:
> > 
> > > cifs: prevent cifs_writepages() from skipping unwritten pages
> > > 
> > > In order to write contiguous pages whenever possible, cifs_writepages()
> > > asks pagevec_lookup_tag() for more pages than it may write at one time.
> > > Normally, it then resets index just past the last page written before calling
> > > pagevec_lookup_tag() again.
> > > 
> > > If cifs_writepages() can't write the first page returned, it wasn't resetting
> > > index, and the next call to pagevec_lookup_tag() resulted in skipping all of
> > > the pages it previously returned, even though cifs_writepages() did nothing
> > > with them.  This can result in data loss when the file descriptor is about
> > > to be closed.
> > > 
> > > This patch ensures that index gets set back to the first returned page so
> > > that none get skipped.
> > > 
> > > Signed-off-by: Dave Kleikamp <shaggy at linux.vnet.ibm.com>
> > > Cc: Shirish S Pargaonkar <shirishp at us.ibm.com>
> > > Cc: Steve French <sfrench at us.ibm.com>
> > > Cc: Jeff Layton <jlayton at redhat.com>
> > > 
> > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> > > index 1540ada..9815e03 100644
> > > --- a/fs/cifs/file.c
> > > +++ b/fs/cifs/file.c
> > > @@ -1404,7 +1404,10 @@ retry:
> > >  			if ((wbc->nr_to_write -= n_iov) <= 0)
> > >  				done = 1;
> > >  			index = next;
> > > -		}
> > > +		} else
> > > +			/* Need to re-find the pages we skipped */
> > > +			index = pvec.pages[0]->index;
> > > +
> > >  		pagevec_release(&pvec);
> > >  	}
> > >  	if (!scanned && !done) {
> > > 
> > 
> > Patch looks sane to me and fairly obvious once you know what you're looking for...
> > 
> > Do we know what condition is occurring to make it break out of the loop before n_iov can be incremented?
> 
> I'm not positive, but I suspect memory pressure is causing pages to be
> written, maybe by pdflush, so a racing thread starts to write the page
> before we lock it.  By the time we lock it and wait for writeback, it's
> clean.

Thanks, makes sense...

Acked-by: Jeff Layton <jlayton at redhat.com>