public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Wenchao Hao <haowenchao@huawei.com>
To: "Martin K. Petersen" <martin.petersen@oracle.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	Mike Christie <michael.christie@oracle.com>,
	"James Bottomley" <jejb@linux.ibm.com>
Cc: <linux-scsi@vger.kernel.org>, <linfeilong@huawei.com>,
	<yanaijie@huawei.com>, <xuhujie@huawei.com>,
	<lijinlin3@huawei.com>
Subject: [QUESTION]: Why did we clear the lowest bit of SCSI command's status in scsi_status_is_good
Date: Mon, 28 Nov 2022 11:58:35 +0800	[thread overview]
Message-ID: <6e9ea80e-d4e0-6d52-47c1-8939c13d60a8@huawei.com> (raw)

static inline bool scsi_status_is_good(int status)                                                                                                                                                             
{
        if (status < 0)
                return false;

        if (host_byte(status) == DID_NO_CONNECT)
                return false;

        /*  
         * FIXME: bit0 is listed as reserved in SCSI-2, but is
         * significant in SCSI-3.  For now, we follow the SCSI-2
         * behaviour and ignore reserved bits.
         */
        status &= 0xfe;
        return ((status == SAM_STAT_GOOD) ||
                (status == SAM_STAT_CONDITION_MET) ||
                /* Next two "intermediate" statuses are obsolete in SAM-4 */
                (status == SAM_STAT_INTERMEDIATE) ||
                (status == SAM_STAT_INTERMEDIATE_CONDITION_MET) ||
                /* FIXME: this is obsolete in SAM-3 */
                (status == SAM_STAT_COMMAND_TERMINATED));
}
We have function defined in include/scsi/scsi.h as above, which is used to check
if the status in result is good.

But the function cleared the lowest bit of SCSI command's status, which would
translate an undefined status to good in some condition, for example the status
is 0x1.

This code seems introduced in this patch:
https://lore.kernel.org/all/1052403312.2097.35.camel@mulgrave/

Is anyone who knows why did we clear the lowest bit? 

We found some firmware or drivers would return status which did not defined in
SAM. Now SCSI middle level do not have an uniform behavior for these undefined
status. I want to change the logic to return error for all status which did not
defined in SAM or define a method in host template to let drivers to judge
what to do in this condition.

             reply	other threads:[~2022-11-28  3:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28  3:58 Wenchao Hao [this message]
2022-11-28 12:52 ` [QUESTION]: Why did we clear the lowest bit of SCSI command's status in scsi_status_is_good James Bottomley
2022-11-28 14:41   ` Wenchao Hao
2022-11-28 15:21     ` James Bottomley
2022-11-28 17:38       ` Wenchao Hao
2022-11-28 18:12         ` James Bottomley
2022-12-02 13:58           ` Wenchao Hao
2022-12-02 14:17             ` James Bottomley
2022-12-02 16:26               ` Wenchao Hao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6e9ea80e-d4e0-6d52-47c1-8939c13d60a8@huawei.com \
    --to=haowenchao@huawei.com \
    --cc=jejb@linux.ibm.com \
    --cc=lijinlin3@huawei.com \
    --cc=linfeilong@huawei.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=michael.christie@oracle.com \
    --cc=xuhujie@huawei.com \
    --cc=yanaijie@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox