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

[Dave Kleikamp]

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.
-- 
David Kleikamp
IBM Linux Technology Center