Re: [PATCH 01/13] cifs: don't fail writepages on -EAGAIN errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff,
Sounds like this patch series is close to ready.  Are you respinning
[only] patch 1 of 13?  Is this the easiest series of yours to
review/merge (into cifs-2.6.git) first for 2.6.38-pre



On Tue, Dec 14, 2010 at 6:18 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Tue, 14 Dec 2010 14:56:18 +0530
> Suresh Jayaraman <sjayaraman@xxxxxxx> wrote:
>
>> On 12/10/2010 09:14 PM, Jeff Layton wrote:
>> > If CIFSSMBWrite2 returns -EAGAIN, then the error should be considered
>> > temporary. CIFS should retry the write instead of setting an error on
>> > the mapping and returning.
>> >
>> > For WB_SYNC_ALL, just retry the write immediately.
>> >
>> > In the WB_SYNC_NONE case, call redirty_page_for_writeback on all of the
>> > pages that didn't get written out and then move on.
>> >
>> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>> > ---
>> >  fs/cifs/file.c |   23 +++++++++++------------
>> >  1 files changed, 11 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> > index fe16f6d..8e57370 100644
>> > --- a/fs/cifs/file.c
>> > +++ b/fs/cifs/file.c
>> > @@ -1430,6 +1430,7 @@ retry:
>> >                             break;
>> >             }
>> >             if (n_iov) {
>> > +retry_write:
>> >                     open_file = find_writable_file(CIFS_I(mapping->host),
>> >                                                     false);
>> >                     if (!open_file) {
>> > @@ -1445,22 +1446,20 @@ retry:
>> >                             cifs_update_eof(cifsi, offset, bytes_written);
>> >                     }
>> >
>> > -                   if (rc || bytes_written < bytes_to_write) {
>> > -                           cERROR(1, "Write2 ret %d, wrote %d",
>> > -                                     rc, bytes_written);
>> > -                           mapping_set_error(mapping, rc);
>> > -                   } else {
>> > +                   /* retry on data-integrity flush */
>> > +                   if (rc == -EAGAIN && wbc->sync_mode == WB_SYNC_ALL)
>> > +                           goto retry_write;
>> > +
>> > +                   if (!rc)
>> >                             cifs_stats_bytes_written(tcon, bytes_written);
>> > -                   }
>> > +                   else if (rc != -EAGAIN)
>> > +                           mapping_set_error(mapping, rc);
>> >
>> >                     for (i = 0; i < n_iov; i++) {
>> >                             page = pvec.pages[first + i];
>> > -                           /* Should we also set page error on
>> > -                           success rc but too little data written? */
>> > -                           /* BB investigate retry logic on temporary
>> > -                           server crash cases and how recovery works
>> > -                           when page marked as error */
>> > -                           if (rc)
>> > +                           if (rc == -EAGAIN)
>> > +                                   redirty_page_for_writepage(wbc, page);
>> > +                           else if (rc != 0)
>> >                                     SetPageError(page);
>> >                             kunmap(page);
>> >                             unlock_page(page);
>>
>> Documentation/filesystems/Locking suggests that rc should be set to 0
>> after redirtying?
>>
>>
>
> Ahh good catch. I'll fix it.
>
> Thanks,
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
>



-- 
Thanks,

Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux