Re: [PATCHv2]sd: Don't treat succeeded SYNC as error

[James Bottomley <jejb@linux.vnet.ibm.com>]

On Tue, 2016-05-10 at 17:46 +0200, Jinpu Wang wrote:
> On Tue, May 10, 2016 at 5:08 PM, James Bottomley
> <jejb@linux.vnet.ibm.com> wrote:
[...]
> > Actually, I think this is symptomatic of a much bigger problem. 
> >  Now that the FS can send zero length non BLOCK_PC request, we're 
> > not treating failure correctly.  blk_update_request() will always
> > finish them becuase they have no bytes outstanding (not having any 
> > in the first case).  So I think we need a special exception for all 
> > zero length commands which complete with a failure to allow them to
> > retry (if required).
> 
> Which request do you mean, I only find FLUSH?

Flush is the only one currently, but zero length fs requests didn't
exist when this bit of SCSI was coded, which is why all the code judges
request completion on have we completed all the bytes (which is
obviously true if you had no bytes to send in the first place).

> I will test your patch.

Thanks.

James

> Thanks,
> Jack
> 
> > 
> > James
> > 
> > ---
> > 
> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > index 8106515..5a97866 100644
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -911,9 +911,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd,
> > unsigned int good_bytes)
> >         }
> > 
> >         /*
> > -        * If we finished all bytes in the request we are done now.
> > +        * special case: failed zero length commands always need to
> > +        * drop down into the retry code. Otherwise, if we finished
> > +        * all bytes in the request we are done now.
> >          */
> > -       if (!scsi_end_request(req, error, good_bytes, 0))
> > +       if (!(good_bytes == 0 && blk_rq_bytes(req) == 0 && result
> > != 0) &&
> > +           !scsi_end_request(req, error, good_bytes, 0))
> >                 return;
> > 
> >         /*
> > 
> 
> 
>