On 2018-02-23 08:33 AM, Martin Wilck wrote:
Gentle reminder - reviews welcome ...
On Tue, 2018-01-30 at 11:25 +0100, Martin Wilck wrote:
Introduce a new blist flag that indicates the device may return
sense code/ASC/ASCQ combinations that indicate different treatment
normal. In particular, some devices need unconditional retry (aka
ADD_TO_MLQUEUE) under certain conditions; otherwise path failures may
falsely detected in the "maybe_retry" case.
Because different devices use different sense codes to indicate this
condition, a single bit is not enough. But we also can't use a bit
every possible status code. Therefore the new flag
just indicates that this is a device for which the return code should
checked. An extra "blacklist" in scsi_aborted_cmd_quirk() then checks
known ASC/ASCQ combinations, and handles them.
When such a combination is encountered for the first time, a message
printed. In systems that have several different peripherals using
flag, that might lead to a wrong match without a warning. This small
is a compromise between exactness and the excessive resources that
required to check for matching device vendor and model name every
Also, if there were different devices (identified by vendor/model)
the same ASC/ASCQ, the code might print stray warnings. But the
effort to required to handle this 100% correctly is hardly justified
the current minimal "blacklist".
I chose to recycle devinfo flag (1<<11) (former BLIST_INQUIRY_58,
since 496c91bbc910) for this purpose rather than extending
64 bit. This could be easily changed of course.
This patch replaces the previously submitted patches
"scsi: handle ABORTED_COMMAND on Fujitsu ETERNUS" and
"scsi: Always retry internal target error" (Hannes Reinecke)
Changes in v2:
- use ARRAY_SIZE (Bart van Assche)
- make blist array static const and use separate bitmask for warned
(Bart van Assche)
- Fix string comparison for SCSI vendor and model
- Print warning at scsi_logging_level 0, and improve message
Signed-off-by: Martin Wilck <mwi...@suse.com>
I checked the code for the ABORTED COMMAND asc=0x10 case. That is a bunch
of Protection Information errors which retrying would be pointless. And
the existing code does check for that just before the new check for
BLIST_ABORTED_CMD_QUIRK. The PI check has the cryptic comment /* DIF */
and we also have 'T10 DIF' appearing in defines. 'DIF' is mentioned _once_
in the T10 drafts thatI can find (in sbc4r15.pdf the last sentence of
section 4.22.1) and is not cross-referenced anywhere that I can find.
So anyway, Martin (P.), could we have and explanation somewhere prominent in
our code what DIF and DIX mean in SCSI (and other standards) terms?
But that is not this patch's problem, so:
Reviewed-by: Douglas Gilbert <dgilb...@interlog.com>