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

[Shirish S Pargaonkar]

Dave Kleikamp <shaggy at linux.vnet.ibm.com> wrote on 11/17/2008 11:57:16 AM:

> On Mon, 2008-11-17 at 11:02 -0600, Shirish S Pargaonkar wrote:
> > 
> > 
> > Dave Kleikamp <shaggy at linux.vnet.ibm.com> wrote on 11/17/2008 09:59:07
> > AM:
> > 
> > > On Mon, 2008-11-17 at 09:42 -0600, Shirish S Pargaonkar wrote:
> > > 
> > > > Shaggy, 
> > > > 
> > > > But if the pdflush grabbed it to write, it would percolate down
> > to 
> > > > calling cifs_writepages right?  If so, how come we do not see
> > those 
> > > > at the server, we would have seen samba server logging those write
> > > > andx 
> > > > and perhaps the offsets would be out of order but there in the
> > samba 
> > > > log file nonetheless! 
> > > > 
> > > You're more familiar with what's been logged, but let me explain
> > what I
> > > think may be happening, and you can tell me if it fits your
> > > observations.
> > > 
> > > The cifs_flush thread gets into cifs_writepages and
> > pagevec_lookup_tag()
> > > returns 14 pages, let's say pages 0-13.
> > >  -----
> > > The pdflush thread writes some smaller number of pages, 0-4.
> > >  -----
> > > After waiting on page 0, the cifs_flush thread sees that it's no
> > longer
> > > dirty, and calls pagevec_lookup_tag() again, but index has been set
> > to
> > > 14, so it gets back pages 14-27.  Pages 5-13 get missed.
> > > -- 
> > > David Kleikamp
> > > IBM Linux Technology Center
> > > 
> > 
> > I see the opposite, i.e. the upper pages are not received by samba
> > server. 
> > For example, after writing 56KB at the 56K boundary, 
> > 8 pages are not received by the samba server but rest of them are. 
> > 
> > Here is an example of what I see... 
> > 
> > This is the log level 10 output on the samba server. 
> > 
> > 
> > real_write_file (39.test): pos = 19267584, size = 57344, returned
> > 57344 
> > real_write_file (39.test): pos = 19324928, size = 57344, returned
> > 57344 
> > real_write_file (39.test): pos = 19382272, size = 57344, returned
> > 57344 
> > real_write_file (39.test): pos = 19439616, size = 57344, returned
> > 57344 
> > real_write_file (39.test): pos = 19496960, size = 57344, returned
> > 57344 
> > real_write_file (39.test): pos = 19587072, size = 57344, returned
> > 57344 
> > real_write_file (39.test): pos = 19644416, size = 57344, returned
> > 57344 
> > real_write_file (39.test): pos = 19701760, size = 57344, returned
> > 57344 
> > 
> > which matches with the hole in the file as the hex dump of the file
> > shows 
> > 
> > 12a5fe0: 6db6 6db6 6db6 6db6 6db6 6db6 6db6 6db6  m.m.m.m.m.m.m.m. 
> > 12a5ff0: 6db6 6db6 6db6 6db6 6db6 6db6 6db6 6db6  m.m.m.m.m.m.m.m. 
> > 12a6000: 0000 0000 0000 0000 0000 0000 0000 0000  ................ 
> > 12a6010: 0000 0000 0000 0000 0000 0000 0000 0000  ................ 
> >                         ......... 
> >                        ......... 
> > 12adfe0: 0000 0000 0000 0000 0000 0000 0000 0000  ................ 
> > 12adff0: 0000 0000 0000 0000 0000 0000 0000 0000  ................ 
> > 12ae000: 6db6 6db6 6db6 6db6 6db6 6db6 6db6 6db6  m.m.m.m.m.m.m.m. 
> > 12ae010: 6db6 6db6 6db6 6db6 6db6 6db6 6db6 6db6  m.m.m.m.m.m.m.m. 
> > 12ae020: 6db6 6db6 6db6 6db6 6db6 6db6 6db6 6db6  m.m.m.m.m.m.m.m. 
> > 
> > I do not see the top 8 pages starting at position 19554304 but see 
> > 14 pages starting at position 19587072 instead. 
> > Upto position 19496960, all position (offsets in write andx) have
> > been 
> > on 56K boundary but not starting at position 19587072 and IMHO that 
> > is because top first 8 pages of 14 pages starting at 19554304 did 
> > never arrive at the samba server. 
> > If so, we could have seen an entry like this in the samba server log 
> >  real_write_file (39.test): pos = 19554304, size = 32768, returned
> > 32768 
> 
> Now I'm confused.  If cifs_writepages breaks out of the for loop for
> some reason other than that the page is already marked non-dirty, then
> why does it resume 8 pages later and not 14?  How does my patch fix it?
> Going back to try the same page succeeds the next time?

Is it possible that pagevec_lookup_tag can return less than the pages
being looked up, i.e. is it possible that pagevec_lookup_tag could return
less (8) dirty pages than the number of pages that were asked (14) when we
are not at the end of the file?

I think this is OK (this is from the previous writes to this file as part
of this testcase) as we are at the end of the file.

  real_write_file (39.test): pos = 24428544, size = 57344, returned 57344
  real_write_file (39.test): pos = 24485888, size = 57344, returned 57344
  real_write_file (39.test): pos = 24543232, size = 32768, returned 32768

> 
> I could see something like this during write-behind, if pages are being
> flushed out while the testcase is still writing to the file, but I would
> expect a later call to cifs_flush when the file is closed to go back and
> clean it up afterwards.
> 
> > Regards, 
> > 
> > Shirish 
> -- 
> David Kleikamp
> IBM Linux Technology Center
> 
-------------- next part --------------
HTML attachment scrubbed and removed