Skip to content

Commit

Permalink
scsi: core: Introduce enum scsi_disposition
Browse files Browse the repository at this point in the history
Improve readability of the code in the SCSI core by introducing an
enumeration type for the values used internally that decide how to continue
processing a SCSI command. The eh_*_handler return values have not been
changed because that would involve modifying all SCSI drivers.

The output of the following command has been inspected to verify that no
out-of-range values are assigned to a variable of type enum
scsi_disposition:

KCFLAGS=-Wassign-enum make CC=clang W=1 drivers/scsi/

Link: https://lore.kernel.org/r/20210415220826.29438-6-bvanassche@acm.org
Cc: Christoph Hellwig <hch@lst.de>
Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
  • Loading branch information
Bart Van Assche authored and Martin K. Petersen committed Apr 16, 2021
1 parent 280e91b commit b8e162f
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 51 deletions.
2 changes: 1 addition & 1 deletion drivers/ata/libata-eh.c
Original file line number Diff line number Diff line change
Expand Up @@ -1599,7 +1599,7 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc,
}

if (qc->flags & ATA_QCFLAG_SENSE_VALID) {
int ret = scsi_check_sense(qc->scsicmd);
enum scsi_disposition ret = scsi_check_sense(qc->scsicmd);
/*
* SUCCESS here means that the sense code could be
* evaluated and should be passed to the upper layers
Expand Down
4 changes: 2 additions & 2 deletions drivers/scsi/device_handler/scsi_dh_alua.c
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,8 @@ static char print_alua_state(unsigned char state)
}
}

static int alua_check_sense(struct scsi_device *sdev,
struct scsi_sense_hdr *sense_hdr)
static enum scsi_disposition alua_check_sense(struct scsi_device *sdev,
struct scsi_sense_hdr *sense_hdr)
{
struct alua_dh_data *h = sdev->handler_data;
struct alua_port_group *pg;
Expand Down
4 changes: 2 additions & 2 deletions drivers/scsi/device_handler/scsi_dh_emc.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@ static int send_trespass_cmd(struct scsi_device *sdev,
return res;
}

static int clariion_check_sense(struct scsi_device *sdev,
struct scsi_sense_hdr *sense_hdr)
static enum scsi_disposition clariion_check_sense(struct scsi_device *sdev,
struct scsi_sense_hdr *sense_hdr)
{
switch (sense_hdr->sense_key) {
case NOT_READY:
Expand Down
4 changes: 2 additions & 2 deletions drivers/scsi/device_handler/scsi_dh_rdac.c
Original file line number Diff line number Diff line change
Expand Up @@ -656,8 +656,8 @@ static blk_status_t rdac_prep_fn(struct scsi_device *sdev, struct request *req)
return BLK_STS_OK;
}

static int rdac_check_sense(struct scsi_device *sdev,
struct scsi_sense_hdr *sense_hdr)
static enum scsi_disposition rdac_check_sense(struct scsi_device *sdev,
struct scsi_sense_hdr *sense_hdr)
{
struct rdac_dh_data *h = sdev->handler_data;

Expand Down
64 changes: 34 additions & 30 deletions drivers/scsi/scsi_error.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ static void scsi_eh_done(struct scsi_cmnd *scmd);
#define HOST_RESET_SETTLE_TIME (10)

static int scsi_eh_try_stu(struct scsi_cmnd *scmd);
static int scsi_try_to_abort_cmd(struct scsi_host_template *,
struct scsi_cmnd *);
static enum scsi_disposition scsi_try_to_abort_cmd(struct scsi_host_template *,
struct scsi_cmnd *);

void scsi_eh_wakeup(struct Scsi_Host *shost)
{
Expand Down Expand Up @@ -151,7 +151,7 @@ scmd_eh_abort_handler(struct work_struct *work)
struct scsi_cmnd *scmd =
container_of(work, struct scsi_cmnd, abort_work.work);
struct scsi_device *sdev = scmd->device;
int rtn;
enum scsi_disposition rtn;

if (scsi_host_eh_past_deadline(sdev->host)) {
SCSI_LOG_ERROR_RECOVERY(3,
Expand Down Expand Up @@ -498,7 +498,7 @@ static void scsi_report_sense(struct scsi_device *sdev,
* When a deferred error is detected the current command has
* not been executed and needs retrying.
*/
int scsi_check_sense(struct scsi_cmnd *scmd)
enum scsi_disposition scsi_check_sense(struct scsi_cmnd *scmd)
{
struct scsi_device *sdev = scmd->device;
struct scsi_sense_hdr sshdr;
Expand All @@ -512,7 +512,7 @@ int scsi_check_sense(struct scsi_cmnd *scmd)
return NEEDS_RETRY;

if (sdev->handler && sdev->handler->check_sense) {
int rc;
enum scsi_disposition rc;

rc = sdev->handler->check_sense(sdev, &sshdr);
if (rc != SCSI_RETURN_NOT_HANDLED)
Expand Down Expand Up @@ -723,7 +723,7 @@ static void scsi_handle_queue_full(struct scsi_device *sdev)
* don't allow for the possibility of retries here, and we are a lot
* more restrictive about what we consider acceptable.
*/
static int scsi_eh_completed_normally(struct scsi_cmnd *scmd)
static enum scsi_disposition scsi_eh_completed_normally(struct scsi_cmnd *scmd)
{
/*
* first check the host byte, to see if there is anything in there
Expand Down Expand Up @@ -804,10 +804,10 @@ static void scsi_eh_done(struct scsi_cmnd *scmd)
* scsi_try_host_reset - ask host adapter to reset itself
* @scmd: SCSI cmd to send host reset.
*/
static int scsi_try_host_reset(struct scsi_cmnd *scmd)
static enum scsi_disposition scsi_try_host_reset(struct scsi_cmnd *scmd)
{
unsigned long flags;
int rtn;
enum scsi_disposition rtn;
struct Scsi_Host *host = scmd->device->host;
struct scsi_host_template *hostt = host->hostt;

Expand All @@ -834,10 +834,10 @@ static int scsi_try_host_reset(struct scsi_cmnd *scmd)
* scsi_try_bus_reset - ask host to perform a bus reset
* @scmd: SCSI cmd to send bus reset.
*/
static int scsi_try_bus_reset(struct scsi_cmnd *scmd)
static enum scsi_disposition scsi_try_bus_reset(struct scsi_cmnd *scmd)
{
unsigned long flags;
int rtn;
enum scsi_disposition rtn;
struct Scsi_Host *host = scmd->device->host;
struct scsi_host_template *hostt = host->hostt;

Expand Down Expand Up @@ -876,10 +876,10 @@ static void __scsi_report_device_reset(struct scsi_device *sdev, void *data)
* timer on it, and set the host back to a consistent state prior to
* returning.
*/
static int scsi_try_target_reset(struct scsi_cmnd *scmd)
static enum scsi_disposition scsi_try_target_reset(struct scsi_cmnd *scmd)
{
unsigned long flags;
int rtn;
enum scsi_disposition rtn;
struct Scsi_Host *host = scmd->device->host;
struct scsi_host_template *hostt = host->hostt;

Expand Down Expand Up @@ -907,9 +907,9 @@ static int scsi_try_target_reset(struct scsi_cmnd *scmd)
* timer on it, and set the host back to a consistent state prior to
* returning.
*/
static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
static enum scsi_disposition scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
{
int rtn;
enum scsi_disposition rtn;
struct scsi_host_template *hostt = scmd->device->host->hostt;

if (!hostt->eh_device_reset_handler)
Expand Down Expand Up @@ -938,8 +938,8 @@ static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
* if the device is temporarily unavailable (eg due to a
* link down on FibreChannel)
*/
static int scsi_try_to_abort_cmd(struct scsi_host_template *hostt,
struct scsi_cmnd *scmd)
static enum scsi_disposition
scsi_try_to_abort_cmd(struct scsi_host_template *hostt, struct scsi_cmnd *scmd)
{
if (!hostt->eh_abort_handler)
return FAILED;
Expand Down Expand Up @@ -1072,8 +1072,8 @@ EXPORT_SYMBOL(scsi_eh_restore_cmnd);
* Return value:
* SUCCESS or FAILED or NEEDS_RETRY
*/
static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
int cmnd_size, int timeout, unsigned sense_bytes)
static enum scsi_disposition scsi_send_eh_cmnd(struct scsi_cmnd *scmd,
unsigned char *cmnd, int cmnd_size, int timeout, unsigned sense_bytes)
{
struct scsi_device *sdev = scmd->device;
struct Scsi_Host *shost = sdev->host;
Expand Down Expand Up @@ -1180,12 +1180,13 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
* that we obtain it on our own. This function will *not* return until
* the command either times out, or it completes.
*/
static int scsi_request_sense(struct scsi_cmnd *scmd)
static enum scsi_disposition scsi_request_sense(struct scsi_cmnd *scmd)
{
return scsi_send_eh_cmnd(scmd, NULL, 0, scmd->device->eh_timeout, ~0);
}

static int scsi_eh_action(struct scsi_cmnd *scmd, int rtn)
static enum scsi_disposition
scsi_eh_action(struct scsi_cmnd *scmd, enum scsi_disposition rtn)
{
if (!blk_rq_is_passthrough(scmd->request)) {
struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd);
Expand Down Expand Up @@ -1238,7 +1239,7 @@ int scsi_eh_get_sense(struct list_head *work_q,
{
struct scsi_cmnd *scmd, *next;
struct Scsi_Host *shost;
int rtn;
enum scsi_disposition rtn;

/*
* If SCSI_EH_ABORT_SCHEDULED has been set, it is timeout IO,
Expand Down Expand Up @@ -1316,7 +1317,8 @@ EXPORT_SYMBOL_GPL(scsi_eh_get_sense);
static int scsi_eh_tur(struct scsi_cmnd *scmd)
{
static unsigned char tur_command[6] = {TEST_UNIT_READY, 0, 0, 0, 0, 0};
int retry_cnt = 1, rtn;
int retry_cnt = 1;
enum scsi_disposition rtn;

retry_tur:
rtn = scsi_send_eh_cmnd(scmd, tur_command, 6,
Expand Down Expand Up @@ -1404,7 +1406,8 @@ static int scsi_eh_try_stu(struct scsi_cmnd *scmd)
static unsigned char stu_command[6] = {START_STOP, 0, 0, 0, 1, 0};

if (scmd->device->allow_restart) {
int i, rtn = NEEDS_RETRY;
int i;
enum scsi_disposition rtn = NEEDS_RETRY;

for (i = 0; rtn == NEEDS_RETRY && i < 2; i++)
rtn = scsi_send_eh_cmnd(scmd, stu_command, 6, scmd->device->request_queue->rq_timeout, 0);
Expand Down Expand Up @@ -1498,7 +1501,7 @@ static int scsi_eh_bus_device_reset(struct Scsi_Host *shost,
{
struct scsi_cmnd *scmd, *bdr_scmd, *next;
struct scsi_device *sdev;
int rtn;
enum scsi_disposition rtn;

shost_for_each_device(sdev, shost) {
if (scsi_host_eh_past_deadline(shost)) {
Expand Down Expand Up @@ -1565,7 +1568,7 @@ static int scsi_eh_target_reset(struct Scsi_Host *shost,

while (!list_empty(&tmp_list)) {
struct scsi_cmnd *next, *scmd;
int rtn;
enum scsi_disposition rtn;
unsigned int id;

if (scsi_host_eh_past_deadline(shost)) {
Expand Down Expand Up @@ -1623,7 +1626,7 @@ static int scsi_eh_bus_reset(struct Scsi_Host *shost,
struct scsi_cmnd *scmd, *chan_scmd, *next;
LIST_HEAD(check_list);
unsigned int channel;
int rtn;
enum scsi_disposition rtn;

/*
* we really want to loop over the various channels, and do this on
Expand Down Expand Up @@ -1694,7 +1697,7 @@ static int scsi_eh_host_reset(struct Scsi_Host *shost,
{
struct scsi_cmnd *scmd, *next;
LIST_HEAD(check_list);
int rtn;
enum scsi_disposition rtn;

if (!list_empty(work_q)) {
scmd = list_entry(work_q->next,
Expand Down Expand Up @@ -1800,9 +1803,9 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd)
* doesn't require the error handler read (i.e. we don't need to
* abort/reset), this function should return SUCCESS.
*/
int scsi_decide_disposition(struct scsi_cmnd *scmd)
enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *scmd)
{
int rtn;
enum scsi_disposition rtn;

/*
* if the device is offline, then we clearly just pass the result back
Expand Down Expand Up @@ -2365,7 +2368,8 @@ scsi_ioctl_reset(struct scsi_device *dev, int __user *arg)
struct Scsi_Host *shost = dev->host;
struct request *rq;
unsigned long flags;
int error = 0, rtn, val;
int error = 0, val;
enum scsi_disposition rtn;

if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
Expand Down
2 changes: 1 addition & 1 deletion drivers/scsi/scsi_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1441,7 +1441,7 @@ static bool scsi_mq_lld_busy(struct request_queue *q)
static void scsi_complete(struct request *rq)
{
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
int disposition;
enum scsi_disposition disposition;

INIT_LIST_HEAD(&cmd->eh_entry);

Expand Down
2 changes: 1 addition & 1 deletion drivers/scsi/scsi_priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ extern void scsi_exit_devinfo(void);
extern void scmd_eh_abort_handler(struct work_struct *work);
extern enum blk_eh_timer_return scsi_times_out(struct request *req);
extern int scsi_error_handler(void *host);
extern int scsi_decide_disposition(struct scsi_cmnd *cmd);
extern enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *cmd);
extern void scsi_eh_wakeup(struct Scsi_Host *shost);
extern void scsi_eh_scmd_add(struct scsi_cmnd *);
void scsi_eh_ready_devs(struct Scsi_Host *shost,
Expand Down
21 changes: 11 additions & 10 deletions include/scsi/scsi.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,16 +180,17 @@ static inline int scsi_is_wlun(u64 lun)
/*
* Internal return values.
*/

#define NEEDS_RETRY 0x2001
#define SUCCESS 0x2002
#define FAILED 0x2003
#define QUEUED 0x2004
#define SOFT_ERROR 0x2005
#define ADD_TO_MLQUEUE 0x2006
#define TIMEOUT_ERROR 0x2007
#define SCSI_RETURN_NOT_HANDLED 0x2008
#define FAST_IO_FAIL 0x2009
enum scsi_disposition {
NEEDS_RETRY = 0x2001,
SUCCESS = 0x2002,
FAILED = 0x2003,
QUEUED = 0x2004,
SOFT_ERROR = 0x2005,
ADD_TO_MLQUEUE = 0x2006,
TIMEOUT_ERROR = 0x2007,
SCSI_RETURN_NOT_HANDLED = 0x2008,
FAST_IO_FAIL = 0x2009,
};

/*
* Midlevel queue return values.
Expand Down
3 changes: 2 additions & 1 deletion include/scsi/scsi_dh.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ struct scsi_device_handler {
/* Filled by the hardware handler */
struct module *module;
const char *name;
int (*check_sense)(struct scsi_device *, struct scsi_sense_hdr *);
enum scsi_disposition (*check_sense)(struct scsi_device *,
struct scsi_sense_hdr *);
int (*attach)(struct scsi_device *);
void (*detach)(struct scsi_device *);
int (*activate)(struct scsi_device *, activate_complete, void *);
Expand Down
2 changes: 1 addition & 1 deletion include/scsi/scsi_eh.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ extern void scsi_report_device_reset(struct Scsi_Host *, int, int);
extern int scsi_block_when_processing_errors(struct scsi_device *);
extern bool scsi_command_normalize_sense(const struct scsi_cmnd *cmd,
struct scsi_sense_hdr *sshdr);
extern int scsi_check_sense(struct scsi_cmnd *);
extern enum scsi_disposition scsi_check_sense(struct scsi_cmnd *);

static inline bool scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr)
{
Expand Down

0 comments on commit b8e162f

Please sign in to comment.