ide-scsi command status bug

ide-scsi command status bug

[Tony Battersby]

Hello,

I have tracked down a problem with incorrect status being returned for a
command sent through sg and ide-scsi.  I am using vanilla 2.4.17.  Here is
what is happening:

The device returns CHECK CONDITION for a command sent to it from sg and
ide-scsi.
The following line executes in idescsi_end_request() in ide-scsi.c:
pc->scsi_cmd->result = (CHECK_CONDITION << 1) | (DID_OK << 16);
ide-scsi calls the callback completion function.
An internal request sense is queued.
The request sense completes successfully.
The following line executes in idescsi_end_request() in ide-scsi.c:
pc->scsi_cmd->result = (DID_OK << 16)
ide-scsi calls the callback completion function for the request sense
command.

In my userspace program, sg_io_hdr_t { status, masked_status } are both
zero, but sg_io_hdr_t { driver_status } & DRIVER_SENSE is set and the sense
data is valid.  It appears to me as if the status from the request sense
command overwrote the original CHECK CONDITION status.  I verified this by
forcing all request sense commands to return a status of 0x01 (reserved bit
set) in idescsi_end_request() in ide-scsi.c, and sure enough, I got a status
of 0x01 for the failed command in sg_io_hdr_t { status }.

I have not tested any other kernel versions.  I am working around the
problem for now by faking a CHECK CONDITION status when driver_status &
DRIVER_SENSE in my program.

Anthony J. Battersby
Cybernetics

Re: ide-scsi command status bug

[Douglas Gilbert]

Tony Battersby wrote:
> 
> Hello,
> 
> I have tracked down a problem with incorrect status being returned for a
> command sent through sg and ide-scsi.  I am using vanilla 2.4.17.  Here is
> what is happening:
> 
> The device returns CHECK CONDITION for a command sent to it from sg and
> ide-scsi.
> The following line executes in idescsi_end_request() in ide-scsi.c:
> pc->scsi_cmd->result = (CHECK_CONDITION << 1) | (DID_OK << 16);
> ide-scsi calls the callback completion function.
> An internal request sense is queued.
> The request sense completes successfully.
> The following line executes in idescsi_end_request() in ide-scsi.c:
> pc->scsi_cmd->result = (DID_OK << 16)
> ide-scsi calls the callback completion function for the request sense
> command.
> 
> In my userspace program, sg_io_hdr_t { status, masked_status } are both
> zero, but sg_io_hdr_t { driver_status } & DRIVER_SENSE is set and the sense
> data is valid.  It appears to me as if the status from the request sense
> command overwrote the original CHECK CONDITION status.  I verified this by
> forcing all request sense commands to return a status of 0x01 (reserved bit
> set) in idescsi_end_request() in ide-scsi.c, and sure enough, I got a status
> of 0x01 for the failed command in sg_io_hdr_t { status }.
> 
> I have not tested any other kernel versions.  I am working around the
> problem for now by faking a CHECK CONDITION status when driver_status &
> DRIVER_SENSE in my program.

Tony,
I tried to document around this bug in the sg driver howto:
	http://tldp.org/HOWTO/SCSI-Generic-HOWTO/x255.html
The ide-scsi driver is probably the only lower level driver that 
has this unfortunate property.

It's a minor dilemma for the sg driver: whether to pass through
this information or correct it on the way passed.

Doug Gilbert

Re: ide-scsi command status bug

[Luben Tuikov]

Tony Battersby wrote:
> 
> Hello,
> 
> I have tracked down a problem with incorrect status being returned for a
> command sent through sg and ide-scsi.  I am using vanilla 2.4.17.  Here is
> what is happening:
> 
> The device returns CHECK CONDITION for a command sent to it from sg and
> ide-scsi.
> The following line executes in idescsi_end_request() in ide-scsi.c:
> pc->scsi_cmd->result = (CHECK_CONDITION << 1) | (DID_OK << 16);
> ide-scsi calls the callback completion function.
> An internal request sense is queued.
> The request sense completes successfully.
> The following line executes in idescsi_end_request() in ide-scsi.c:
> pc->scsi_cmd->result = (DID_OK << 16)
> ide-scsi calls the callback completion function for the request sense
> command.
> 
> In my userspace program, sg_io_hdr_t { status, masked_status } are both
> zero, but sg_io_hdr_t { driver_status } & DRIVER_SENSE is set and the sense
> data is valid.  It appears to me as if the status from the request sense
> command overwrote the original CHECK CONDITION status.  I verified this by
> forcing all request sense commands to return a status of 0x01 (reserved bit
> set) in idescsi_end_request() in ide-scsi.c, and sure enough, I got a status
> of 0x01 for the failed command in sg_io_hdr_t { status }.

You are relying on specific Linux SCSI core behaviour
rather than the spec. SPC-2, 7.20.1, specifies that if REQUEST
SENSE completed successfully, i.e. the sense data is valid, then
status GOOD is returned. If the status is CHECK CONDITION
for a completed REQUEST SENSE command then the sense data may NOT
be valid.

Simply: if you issue REQUEST SENSE and the command completes and
returns valid sense data, then you should get GOOD status.

You should NOT filter REQUEST SENSE and set status to CHECK CONDITION!
 
> I have not tested any other kernel versions.  I am working around the
> problem for now by faking a CHECK CONDITION status when driver_status &
> DRIVER_SENSE in my program.

In userspace you can pretty much make up your own rules.

-- 
Luben

RE: ide-scsi command status bug

[Tony Battersby]

> Luben Tuikov wrote:
>
> You are relying on specific Linux SCSI core behaviour
> rather than the spec. SPC-2, 7.20.1, specifies that if REQUEST
> SENSE completed successfully, i.e. the sense data is valid, then
> status GOOD is returned. If the status is CHECK CONDITION
> for a completed REQUEST SENSE command then the sense data may NOT
> be valid.

Yes, but there are two SCSI commands involved here.  There is the command
that I sent to the sg driver, which got CHECK CONDITION status, and then
there is the auto-generated Request Sense command, which got GOOD status.
Most lowlevel drivers return the CHECK CONDITION from the original command
as the status (sr_result & 0xff), which is probably the behavior most people
would expect.  The ide-scsi driver returns the GOOD status from the Request
Sense instead.

A brief look over the code indicates that this behavior is the result of the
ide-scsi driver relying on the midlevel to generate the Request Sense rather
than automatically retrieving the sense data itself.  The ide-scsi driver
returns GOOD status for the Request Sense (which is certainly the correct
thing to do), and the midlevel overwrites the original CHECK CONDITION with
the GOOD status from the Request Sense.  Looks more to be a design flaw in
the midlevel, but it only shows up with ide-scsi since other lowlevel
drivers retrieve the sense data themselves and don't rely on the midlevel
for auto-sense.

Obviously, there needs to be a way for an upper-level driver or userspace
app to tell that the original command completed with an error.  I was using
sg_io_hdr_t { status } with other lowlevel drivers, but with the ide-scsi
driver I had to check sg_io_hdr_t { driver_status } & DRIVER_SENSE instead.

> Simply: if you issue REQUEST SENSE and the command completes and
> returns valid sense data, then you should get GOOD status.

Yes, of course.  This is what I expect if I send an explicit Request Sense
to the sg driver (unsolicited sense).

I would also expect that if the internal Request Sense that is automatically
generated in response to a CHECK CONDITION itself gets a CHECK CONDITION,
then some other error code would be returned in sg_io_hdr_t {
driver_status } or sg_io_hdr_t { host_status } to indicate that the sense
data could not be retrieved.

Anthony J. Battersby
Cybernetics