Skip to content

Commit f27db8e

Browse files
Finn Thainmartinkpetersen
Finn Thain
authored and
martinkpetersen
committed
ncr5380: Fix autosense bugs
NCR5380_information_transfer() may re-queue a command for autosense, after calling scsi_eh_prep_cmnd(). This creates several possibilities: 1. Reselection may intervene before the re-queued command gets processed. If the reconnected command then undergoes autosense, this causes the scsi_eh_save data from the previous command to be overwritten. 2. After NCR5380_information_transfer() calls scsi_eh_prep_cmnd(), a new REQUEST SENSE command may arrive. This would be queued ahead of any command already undergoing autosense, which means the scsi_eh_save data might be restored to the wrong command. 3. After NCR5380_information_transfer() calls scsi_eh_prep_cmnd(), eh_abort_handler() may abort the command. But the scsi_eh_save data is not discarded, which means the scsi_eh_save data might be incorrectly restored to the next REQUEST SENSE command issued. This patch adds a new autosense list so that commands that are re-queued because of a CHECK CONDITION result can be kept apart from the REQUEST SENSE commands that arrive via queuecommand. This patch also adds a function dedicated to dequeueing and preparing the next command for processing. By refactoring the main loop in this way, scsi_eh_save takes place when an autosense command is dequeued rather than when re-queued. Signed-off-by: Finn Thain <fthain@telegraphics.com.au> Reviewed-by: Hannes Reinecke <hare@suse.com> Tested-by: Ondrej Zary <linux@rainbow-software.org> Tested-by: Michael Schmitz <schmitzmic@gmail.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 677e019 commit f27db8e

File tree

3 files changed

+249
-186
lines changed

3 files changed

+249
-186
lines changed

‎drivers/scsi/NCR5380.c

Lines changed: 111 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,9 @@ static inline void initialize_SCp(struct scsi_cmnd *cmd)
244244
cmd->SCp.ptr = NULL;
245245
cmd->SCp.this_residual = 0;
246246
}
247+
248+
cmd->SCp.Status = 0;
249+
cmd->SCp.Message = 0;
247250
}
248251

249252
/**
@@ -622,6 +625,8 @@ static int NCR5380_init(struct Scsi_Host *instance, int flags)
622625
#endif
623626
spin_lock_init(&hostdata->lock);
624627
hostdata->connected = NULL;
628+
hostdata->sensing = NULL;
629+
INIT_LIST_HEAD(&hostdata->autosense);
625630
INIT_LIST_HEAD(&hostdata->unissued);
626631
INIT_LIST_HEAD(&hostdata->disconnected);
627632

@@ -738,6 +743,16 @@ static void complete_cmd(struct Scsi_Host *instance,
738743

739744
dsprintk(NDEBUG_QUEUES, instance, "complete_cmd: cmd %p\n", cmd);
740745

746+
if (hostdata->sensing == cmd) {
747+
/* Autosense processing ends here */
748+
if ((cmd->result & 0xff) != SAM_STAT_GOOD) {
749+
scsi_eh_restore_cmnd(cmd, &hostdata->ses);
750+
set_host_byte(cmd, DID_ERROR);
751+
} else
752+
scsi_eh_restore_cmnd(cmd, &hostdata->ses);
753+
hostdata->sensing = NULL;
754+
}
755+
741756
hostdata->busy[scmd_id(cmd)] &= ~(1 << cmd->device->lun);
742757

743758
cmd->scsi_done(cmd);
@@ -797,6 +812,64 @@ static int NCR5380_queue_command(struct Scsi_Host *instance,
797812
return 0;
798813
}
799814

815+
/**
816+
* dequeue_next_cmd - dequeue a command for processing
817+
* @instance: the scsi host instance
818+
*
819+
* Priority is given to commands on the autosense queue. These commands
820+
* need autosense because of a CHECK CONDITION result.
821+
*
822+
* Returns a command pointer if a command is found for a target that is
823+
* not already busy. Otherwise returns NULL.
824+
*/
825+
826+
static struct scsi_cmnd *dequeue_next_cmd(struct Scsi_Host *instance)
827+
{
828+
struct NCR5380_hostdata *hostdata = shost_priv(instance);
829+
struct NCR5380_cmd *ncmd;
830+
struct scsi_cmnd *cmd;
831+
832+
if (list_empty(&hostdata->autosense)) {
833+
list_for_each_entry(ncmd, &hostdata->unissued, list) {
834+
cmd = NCR5380_to_scmd(ncmd);
835+
dsprintk(NDEBUG_QUEUES, instance, "dequeue: cmd=%p target=%d busy=0x%02x lun=%llu\n",
836+
cmd, scmd_id(cmd), hostdata->busy[scmd_id(cmd)], cmd->device->lun);
837+
838+
if (!(hostdata->busy[scmd_id(cmd)] & (1 << cmd->device->lun))) {
839+
list_del(&ncmd->list);
840+
dsprintk(NDEBUG_QUEUES, instance,
841+
"dequeue: removed %p from issue queue\n", cmd);
842+
return cmd;
843+
}
844+
}
845+
} else {
846+
/* Autosense processing begins here */
847+
ncmd = list_first_entry(&hostdata->autosense,
848+
struct NCR5380_cmd, list);
849+
list_del(&ncmd->list);
850+
cmd = NCR5380_to_scmd(ncmd);
851+
dsprintk(NDEBUG_QUEUES, instance,
852+
"dequeue: removed %p from autosense queue\n", cmd);
853+
scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
854+
hostdata->sensing = cmd;
855+
return cmd;
856+
}
857+
return NULL;
858+
}
859+
860+
static void requeue_cmd(struct Scsi_Host *instance, struct scsi_cmnd *cmd)
861+
{
862+
struct NCR5380_hostdata *hostdata = shost_priv(instance);
863+
struct NCR5380_cmd *ncmd = scsi_cmd_priv(cmd);
864+
865+
if (hostdata->sensing) {
866+
scsi_eh_restore_cmnd(cmd, &hostdata->ses);
867+
list_add(&ncmd->list, &hostdata->autosense);
868+
hostdata->sensing = NULL;
869+
} else
870+
list_add(&ncmd->list, &hostdata->unissued);
871+
}
872+
800873
/**
801874
* NCR5380_main - NCR state machines
802875
*
@@ -814,63 +887,40 @@ static void NCR5380_main(struct work_struct *work)
814887
struct NCR5380_hostdata *hostdata =
815888
container_of(work, struct NCR5380_hostdata, main_task);
816889
struct Scsi_Host *instance = hostdata->host;
817-
struct NCR5380_cmd *ncmd, *n;
890+
struct scsi_cmnd *cmd;
818891
int done;
819892

820893
spin_lock_irq(&hostdata->lock);
821894
do {
822895
done = 1;
823896

824-
if (!hostdata->connected) {
825-
dprintk(NDEBUG_MAIN, "scsi%d : not connected\n", instance->host_no);
826-
/*
827-
* Search through the issue_queue for a command destined
828-
* for a target that's not busy.
829-
*/
830-
list_for_each_entry_safe(ncmd, n, &hostdata->unissued,
831-
list) {
832-
struct scsi_cmnd *tmp = NCR5380_to_scmd(ncmd);
833-
834-
dsprintk(NDEBUG_QUEUES, instance, "main: tmp=%p target=%d busy=%d lun=%llu\n",
835-
tmp, scmd_id(tmp), hostdata->busy[scmd_id(tmp)],
836-
tmp->device->lun);
837-
/* When we find one, remove it from the issue queue. */
838-
if (!(hostdata->busy[tmp->device->id] &
839-
(1 << (u8)(tmp->device->lun & 0xff)))) {
840-
list_del(&ncmd->list);
841-
dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
842-
instance, "main: removed %p from issue queue\n",
843-
tmp);
897+
while (!hostdata->connected &&
898+
(cmd = dequeue_next_cmd(instance))) {
844899

845-
/*
846-
* Attempt to establish an I_T_L nexus here.
847-
* On success, instance->hostdata->connected is set.
848-
* On failure, we must add the command back to the
849-
* issue queue so we can keep trying.
850-
*/
851-
/*
852-
* REQUEST SENSE commands are issued without tagged
853-
* queueing, even on SCSI-II devices because the
854-
* contingent allegiance condition exists for the
855-
* entire unit.
856-
*/
900+
dsprintk(NDEBUG_MAIN, instance, "main: dequeued %p\n", cmd);
857901

858-
if (!NCR5380_select(instance, tmp)) {
859-
/* OK or bad target */
860-
} else {
861-
/* Need to retry */
862-
list_add(&ncmd->list, &hostdata->unissued);
863-
dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
864-
instance, "main: select() failed, %p returned to issue queue\n",
865-
tmp);
866-
done = 0;
867-
}
868-
if (hostdata->connected)
869-
break;
870-
} /* if target/lun is not busy */
871-
} /* for */
872-
} /* if (!hostdata->connected) */
902+
/*
903+
* Attempt to establish an I_T_L nexus here.
904+
* On success, instance->hostdata->connected is set.
905+
* On failure, we must add the command back to the
906+
* issue queue so we can keep trying.
907+
*/
908+
/*
909+
* REQUEST SENSE commands are issued without tagged
910+
* queueing, even on SCSI-II devices because the
911+
* contingent allegiance condition exists for the
912+
* entire unit.
913+
*/
873914

915+
if (!NCR5380_select(instance, cmd)) {
916+
dsprintk(NDEBUG_MAIN, instance, "main: selected target %d for command %p\n",
917+
scmd_id(cmd), cmd);
918+
} else {
919+
dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES, instance,
920+
"main: select failed, returning %p to queue\n", cmd);
921+
requeue_cmd(instance, cmd);
922+
}
923+
}
874924
if (hostdata->connected
875925
#ifdef REAL_DMA
876926
&& !hostdata->dmalen
@@ -1853,43 +1903,21 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) {
18531903

18541904
hostdata->connected = NULL;
18551905

1856-
/*
1857-
* I'm not sure what the correct thing to do here is :
1858-
*
1859-
* If the command that just executed is NOT a request
1860-
* sense, the obvious thing to do is to set the result
1861-
* code to the values of the stored parameters.
1862-
*
1863-
* If it was a REQUEST SENSE command, we need some way
1864-
* to differentiate between the failure code of the original
1865-
* and the failure code of the REQUEST sense - the obvious
1866-
* case is success, where we fall through and leave the result
1867-
* code unchanged.
1868-
*
1869-
* The non-obvious place is where the REQUEST SENSE failed
1870-
*/
1871-
1872-
if (cmd->cmnd[0] != REQUEST_SENSE)
1873-
cmd->result = cmd->SCp.Status | (cmd->SCp.Message << 8);
1874-
else if (status_byte(cmd->SCp.Status) != GOOD)
1875-
cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
1876-
1877-
if ((cmd->cmnd[0] == REQUEST_SENSE) &&
1878-
hostdata->ses.cmd_len) {
1879-
scsi_eh_restore_cmnd(cmd, &hostdata->ses);
1880-
hostdata->ses.cmd_len = 0 ;
1881-
}
1882-
1883-
if ((cmd->cmnd[0] != REQUEST_SENSE) && (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
1884-
scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
1906+
cmd->result &= ~0xffff;
1907+
cmd->result |= cmd->SCp.Status;
1908+
cmd->result |= cmd->SCp.Message << 8;
18851909

1886-
list_add(&ncmd->list, &hostdata->unissued);
1887-
dsprintk(NDEBUG_AUTOSENSE | NDEBUG_QUEUES,
1888-
instance, "REQUEST SENSE cmd %p added to head of issue queue\n",
1889-
cmd);
1890-
hostdata->busy[cmd->device->id] &= ~(1 << (cmd->device->lun & 0xFF));
1891-
} else {
1910+
if (cmd->cmnd[0] == REQUEST_SENSE)
18921911
complete_cmd(instance, cmd);
1912+
else {
1913+
if (cmd->SCp.Status == SAM_STAT_CHECK_CONDITION ||
1914+
cmd->SCp.Status == SAM_STAT_COMMAND_TERMINATED) {
1915+
dsprintk(NDEBUG_QUEUES, instance, "autosense: adding cmd %p to tail of autosense queue\n",
1916+
cmd);
1917+
list_add_tail(&ncmd->list,
1918+
&hostdata->autosense);
1919+
} else
1920+
complete_cmd(instance, cmd);
18931921
}
18941922

18951923
/*

‎drivers/scsi/NCR5380.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,12 @@ struct NCR5380_hostdata {
256256
unsigned char last_message; /* last message OUT */
257257
struct scsi_cmnd *connected; /* currently connected cmnd */
258258
struct list_head unissued; /* waiting to be issued */
259+
struct list_head autosense; /* priority issue queue */
259260
struct list_head disconnected; /* waiting for reconnect */
260261
spinlock_t lock; /* protects this struct */
261262
int flags;
262263
struct scsi_eh_save ses;
264+
struct scsi_cmnd *sensing;
263265
char info[256];
264266
int read_overruns; /* number of bytes to cut from a
265267
* transfer to handle chip overruns */

0 commit comments

Comments
 (0)