Our qual rig is having some woes. I'll let you know how the test works
out after I finish resuscitating it.
On Mon, Jun 30, 2014 at 11:44 AM, Steven Haber <steven@xxxxxxxxxx> wrote:
> Our qual rig is having some woes. I'll let you know how the test works out
> after I finish resuscitating it.
>
>
> On Fri, Jun 27, 2014 at 12:55 AM, Christoph Hellwig <hch@xxxxxxxxxxxxx>
> wrote:
>>
>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
