Re: fsx-linux failing with latest cifs-2.6 git tree

[Jeff Layton <jlayton@redhat.com>]

On Fri, 21 Nov 2008 14:38:18 -0600
"Steve French" <smfrench@gmail.com> wrote:

> Fix attached.
> 
> Shaggy/Jeff/Nick etc. do you want to review/ack it since it is late in the rc?
> 
> On Fri, Nov 21, 2008 at 2:18 PM, Steve French <smfrench@gmail.com> wrote:
> > Looks like the following change to cifs_write_begin does fix it ...
> > thanks Shaggy ...
> >
> > @@ -2062,8 +2074,10 @@ static int cifs_write_begin(struct file *file,
> > struct address_space *mapping,
> >  {
> >        pgoff_t index = pos >> PAGE_CACHE_SHIFT;
> >        loff_t offset = pos & (PAGE_CACHE_SIZE - 1);
> > +       loff_t page_start = pos & PAGE_MASK;
> >
> >       cFYI(1, ("write_begin from %lld len %d", (long long)pos, len));
> > @@ -2081,13 +2095,14 @@ static int cifs_write_begin(struct file *file,
> > struct address_space *mapping,
> >                int rc;
> >
> >                /* might as well read a page, it is fast enough */
> > -               rc = cifs_readpage_worker(file, *pagep, &offset);
> > +               rc = cifs_readpage_worker(file, *pagep, &page_start);
> >
> >                /* we do not need to pass errors back
> >                   e.g. if we do not have read access to the file
> >                   because cifs_write_end will attempt synchronous writes
> >                   -- shaggy */
> >
> >
> > On Fri, Nov 21, 2008 at 1:41 PM, Dave Kleikamp
> > <shaggy@linux.vnet.ibm.com> wrote:
> >> On Fri, 2008-11-21 at 13:13 -0600, Steve French wrote:
> >>> Looks like this section of code is wrong in cifs_write_begin:
> >>>
> >>>         if ((file->f_flags & O_ACCMODE) != O_WRONLY) {
> >>>                 int rc;
> >>>
> >>>                 /* might as well read a page, it is fast enough */
> >>>                 rc = cifs_readpage_worker(file, *pagep, &offset);
> >>>
> >>>                 /* we do not need to pass errors back
> >>>                    e.g. if we do not have read access to the file
> >>>                    because cifs_write_end will attempt synchronous writes
> >>>                    -- shaggy */
> >>>
> >>>
> >>> We see a case in which a write begins at offset 0x2e42f but the range
> >>> immediately before it is unitialized in write_begin
> >>>
> >>> shouldn't we be doing a read of the whole page?
> >>
> >> What cifs_write_begin() passes in as offset is completely wrong.  It
> >> should be the file offset of the beginning of the page rather than some
> >> offset within the page.
> >> --
> >> David Kleikamp
> >> IBM Linux Technology Center
> >>

Good catch...

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