Re: [PATCH 16/71] ncr5380: Proceed with next command after NCR5380_select() calls scsi_done

[Hannes Reinecke]

On 11/18/2015 09:35 AM, Finn Thain wrote:
> If a target disappears from the SCSI bus, NCR5380_select() may
> subsequently fail with a time-out. In this situation, scsi_done is
> called and NCR5380_select() returns 0. Both hostdata->connected and
> hostdata->selecting are NULL and the main loop should proceed with
> the next command in the issue queue. Clarify this logic.
> 
> Signed-off-by: Finn Thain 
> 
> ---
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 16/71] ncr5380: Proceed with next command after NCR5380_select() calls scsi_done

[Finn Thain]

If a target disappears from the SCSI bus, NCR5380_select() may
subsequently fail with a time-out. In this situation, scsi_done is
called and NCR5380_select() returns 0. Both hostdata->connected and
hostdata->selecting are NULL and the main loop should proceed with
the next command in the issue queue. Clarify this logic.

Signed-off-by: Finn Thain 

---

Good code style would be,

if (this) {
/* do stuff */
}

rather than,

if (!this) {
} else {
/* do stuff */
}

But I've used the latter form at this point in the series for a cleaner
diff between NCR5380.c and atari_NC5380.c, making it easier to gradually
reduce discrepancies. Code style gets fixed up along the way.

---
 drivers/scsi/NCR5380.c   |8 ++--
 drivers/scsi/atari_NCR5380.c |8 
 2 files changed, 10 insertions(+), 6 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-11-18 19:33:21.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-11-18 19:33:23.0 +1100
@@ -1007,14 +1007,18 @@ static void NCR5380_main(struct work_str
 */
 
if (!NCR5380_select(instance, tmp)) {
-   break;
+   /* OK or bad target */
} else {
+   /* Need to retry */
LIST(tmp, 
hostdata->issue_queue);
tmp->host_scribble = (unsigned 
char *) hostdata->issue_queue;
hostdata->issue_queue = tmp;
done = 0;

dprintk(NDEBUG_MAIN|NDEBUG_QUEUES, "scsi%d : main(): select() failed, returned 
to issue_queue\n", instance->host_no);
}
+   if (hostdata->connected ||
+   hostdata->selecting)
+   break;
/* lock held here still */
}   /* if target/lun is not busy */
}   /* for */
@@ -1024,7 +1028,7 @@ static void NCR5380_main(struct work_str
tmp = (struct scsi_cmnd *) hostdata->selecting;
/* Selection will drop and retake the lock */
if (!NCR5380_select(instance, tmp)) {
-   /* Ok ?? */
+   /* OK or bad target */
} else {
/* RvC: device failed, so we wait a long time
   this is needed for Mustek scanners, that
Index: linux/drivers/scsi/atari_NCR5380.c
===
--- linux.orig/drivers/scsi/atari_NCR5380.c 2015-11-18 19:33:21.0 
+1100
+++ linux/drivers/scsi/atari_NCR5380.c  2015-11-18 19:33:23.0 +1100
@@ -1139,13 +1139,13 @@ static void NCR5380_main(struct work_str
cmd_get_tag(tmp, tmp->cmnd[0] != 
REQUEST_SENSE);
 #endif
if (!NCR5380_select(instance, tmp)) {
+   /* OK or bad target */
local_irq_disable();
hostdata->retain_dma_intr--;
-   /* release if target did not 
response! */
maybe_release_dma_irq(instance);
local_irq_restore(flags);
-   break;
} else {
+   /* Need to retry */
local_irq_disable();
LIST(tmp, 
hostdata->issue_queue);
SET_NEXT(tmp, 
hostdata->issue_queue);
@@ -1157,9 +1157,9 @@ static void NCR5380_main(struct work_str
local_irq_restore(flags);
dprintk(NDEBUG_MAIN, "scsi%d: 
main(): select() failed, "
"returned to 
issue_queue\n", HOSTNO);
-   if (hostdata->connected)
-   break;
}
+