On 10/27/2014 06:26 AM, Finn Thain wrote:
> Simplify falcon_release_lock_if_possible() by making callers responsible for
> disabling local IRQ's, which they must do anyway to correctly synchronize
> the ST DMA "lock" with core driver data structures. Move this
> synchronization logic to the core driver with which it is tightly coupled.
>
> Other LLD's like sun3_scsi and mac_scsi that can make use of this core
> driver can just stub out the NCR5380_acquire_dma_irq() and
> NCR5380_release_dma_irq() calls so the compiler will eliminate the
> ST DMA code.
>
> Remove a redundant local_irq_save/restore pair (irq's are disabled for
> interrupt handlers these days). Revise the locking for
> atari_scsi_bus_reset(): use local_irq_save/restore() instead of
> atari_turnoff/turnon_irq(). There is no guarantee that atari_scsi still
> holds the ST DMA lock during EH, so atari_turnoff/turnon_irq() could
> end up dropping an IDE or floppy interrupt.
>
> Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx>
>
> ---
> drivers/scsi/atari_NCR5380.c | 72 ++++++++++++++++++++++++++-----------------
> drivers/scsi/atari_scsi.c | 47 ++++++++++------------------
> 2 files changed, 62 insertions(+), 57 deletions(-)
>
> Index: linux/drivers/scsi/atari_NCR5380.c
> ===================================================================
> --- linux.orig/drivers/scsi/atari_NCR5380.c 2014-10-27 16:25:54.000000000 +1100
> +++ linux/drivers/scsi/atari_NCR5380.c 2014-10-27 16:25:55.000000000 +1100
> @@ -926,7 +926,7 @@ static int NCR5380_queue_command(struct
> * alter queues and touch the lock.
> */
> /* perhaps stop command timer here */
> - if (!falcon_get_lock())
> + if (!NCR5380_acquire_dma_irq(instance))
> return SCSI_MLQUEUE_HOST_BUSY;
> /* perhaps restart command timer here */
>
> @@ -962,6 +962,18 @@ static int NCR5380_queue_command(struct
> return 0;
> }
>
> +static inline void maybe_release_dma_irq(struct Scsi_Host *instance)
> +{
> + struct NCR5380_hostdata *hostdata = shost_priv(instance);
> +
> + /* Caller does the locking needed to set & test these data atomically */
> + if (!hostdata->disconnected_queue &&
> + !hostdata->issue_queue &&
> + !hostdata->connected &&
> + !hostdata->retain_dma_intr)
> + NCR5380_release_dma_irq(instance);
> +}
> +
> /*
> * Function : NCR5380_main (void)
> *
> @@ -1084,9 +1096,11 @@ static void NCR5380_main(struct work_str
> cmd_get_tag(tmp, tmp->cmnd[0] != REQUEST_SENSE);
> #endif
> if (!NCR5380_select(instance, tmp)) {
> + local_irq_disable();
> hostdata->retain_dma_intr--;
> /* release if target did not response! */
> - falcon_release_lock_if_possible(hostdata);
> + maybe_release_dma_irq(instance);
> + local_irq_restore(flags);
> break;
> } else {
> local_irq_disable();
> @@ -2085,11 +2099,12 @@ static void NCR5380_information_transfer
> case COMMAND_COMPLETE:
> /* Accept message by clearing ACK */
> NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE);
> - /* ++guenther: possible race with Falcon locking */
> - hostdata->retain_dma_intr++;
> - hostdata->connected = NULL;
> dprintk(NDEBUG_QUEUES, "scsi%d: command for target %d, lun %llu "
> "completed\n", HOSTNO, cmd->device->id, cmd->device->lun);
> +
> + local_irq_save(flags);
> + hostdata->retain_dma_intr++;
> + hostdata->connected = NULL;
> #ifdef SUPPORT_TAGS
> cmd_free_tag(cmd);
> if (status_byte(cmd->SCp.Status) == QUEUE_FULL) {
> @@ -2148,17 +2163,17 @@ static void NCR5380_information_transfer
>
> dprintk(NDEBUG_AUTOSENSE, "scsi%d: performing request sense\n", HOSTNO);
>
> - local_irq_save(flags);
> LIST(cmd,hostdata->issue_queue);
> SET_NEXT(cmd, hostdata->issue_queue);
> hostdata->issue_queue = (struct scsi_cmnd *) cmd;
> - local_irq_restore(flags);
> dprintk(NDEBUG_QUEUES, "scsi%d: REQUEST SENSE added to head of "
> "issue queue\n", H_NO(cmd));
> } else {
> cmd->scsi_done(cmd);
> }
>
> + local_irq_restore(flags);
> +
> NCR5380_write(SELECT_ENABLE_REG, hostdata->id_mask);
> /*
> * Restore phase bits to 0 so an interrupted selection,
Probably a stupid question, but you're disabling local interrupts.
scsi_done() OTOH is triggering a soft irq. Does this work?
Shouldn't we enable interrupts before calling scsi_done() ?
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
