Re: [PATCH 2/2] scsi : fixing the new host byte settings (DID_TARGET_FAILURE and DID_NEXUS_FAILURE)

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

Hi Babu,

Thanks for finding this.

On Tue, Jan 24 2012 at  3:38pm -0500,
Moger, Babu <Babu.Moger@xxxxxxxxxx> wrote:

> Resubmitting as my previous post had format issues and did not go linux-scsi.
> This patch fixes the host byte settings DID_TARGET_FAILURE and DID_NEXUS_FAILURE.
> The function __scsi_error_from_host_byte,  tries to reset the host byte to DID_OK. But that
> does not happen because of the OR operation. 
> Here is the flow.
> scsi_softirq_done-> scsi_decide_disposition -> __scsi_error_from_host_byte

or more accurately:

scsi_softirq_done -> scsi_decide_disposition
scsi_softirq_done -> scsi_finish_command -> scsi_io_completion -> __scsi_error_from_host_byte

> Let's take an example with DID_NEXUS_FAILURE. In scsi_decide_disposition, result will be set as
> DID_NEXUS_FAILURE (=0x11). Then in __scsi_error_from_host_byte, when we do OR with 
> DID_OK.  Purpose is to reset it back to DID_OK. But that does not happen.  This patch fixes this issue. 

We clearly aren't properly resetting to DID_OK but I'm not seeing an
obvious "nasty bug" that is lurking due to this.  Am I missing

__scsi_error_from_host_byte() is setting error which is passed back up
via blk_end_request() and blk_end_request_all().  And in my previous
testing I know that corresponding errors are making it out to
dm-multipath (e.g. -EREMOTEIO).

Also, your patch header is missing the location where DID_OK is not
properly matched (because it wasn't set exclussively due to being
or'd).  Looks like scsi_noretry_cmd() will be made more efficient
because it will match DID_OK immediately.  Any other locations?  Would
be good to call them out.


dm-devel mailing list

[DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]     [Fedora Docs]

Add to Google Powered by Linux