On Thu, Jun 26, 2014 at 02:52:14PM -0400, James Bottomley wrote:
> > +   } else if (req->cmd_flags & REQ_FLUSH) {
> > +           /*
> > +            * Flush request don't transfer data, so we can't try
> > +            * to complete the good bytes first before checking
> > +            * the error.
> > +            */
> > +           if (result && !sense_deferred)
> > +                   error = __scsi_error_from_host_byte(cmd, result);
> 
> Well, no, that's wrong.  To catch all of the corner cases that slip
> through here, the check is for a zero length command with and error
> otherwise we get the same problem for failed discards.

It's correct for the current code base where FLUSH is the only zero
length !BLOCK_PC case.  Your version is fine too, except for the
incorrect comment:

> +     } else if (blk_rq_bytes(req) == 0 && result && !sense_deferred) {
> +             /*
> +              * Certain non BLOCK_PC requests are commands that don't
> +              * actually transfer anything (FLUSH, DISCARD), so cannot use
> +              * good_bytes == 0 as the signal for an error.  This sets the
> +              * error explicitly for the problem case
> +              */

There aren't BLOCK_PC commands, otherwise they wouldn't end up here.

Note that REQ_DISCARD has been rewritten into either an WRITE_SAME or
UNMAP at this point, which always transfer data.

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

Reply via email to