> Module Name: src
> Committed By: nat
> Date: Mon Oct 28 14:36:43 UTC 2024
>
> Modified Files:
> src/sys/dev/ic: ncr5380sbc.c
> src/sys/dev/scsipi: scsipi_base.c scsipiconf.h
>
> Log Message:
> Introduce scsipi_done_once.
>
> This allows for transfers to be sucessfully aborted on the ncr5380sbc(4).
>
> This may be usefull in future for other scsi controllers.
I'm confused, didn't we conclude this change has to be unnecessary
months ago?
It is very weird that such a change to the fundamental control flow of
scsi transfers is needed for one particular driver, which makes me
strongly suspect something is wrong with that driver in particular --
or there's something wrong with the scsi subsystem's control flow
itself, which is _really important_ to understand before we flail
around with hacks in the driver-independent scsi subsystem, because
whatever the issue is might affect all drivers.
> /* Tell common SCSI code it is done. */
> - scsipi_done(xs);
> + scsipi_done_once(xs);
>
> sc->sc_state = NCR_IDLE;
> /* Now ncr5380_sched() may be called again. */
> +
> + /* Check the queue. */
> + scsipi_channel_thaw(&sc->sc_channel, 0);
Why isn't it enough to do this, with or without the aborting
conditional?
+ const bool aborting = sc->sc_state & NCR_ABORTING;
+
/* Tell common SCSI code it is done. */
+ if (aborting)
+ scsipi_channel_freeze(&sc->sc_channel, 1);
scsipi_done(xs);
sc->sc_state = NCR_IDLE;
/* Now ncr5380_sched() may be called again. */
+
+ /* Check the queue if we were aborting. */
+ if (aborting)
+ scsipi_channel_thaw(&sc->sc_channel, 1);
The logic in scsipi_done now is essentially:
scsipi_done_once(...);
for (;;) /* scspi_run_queue */
mutex_enter(chan_mtx(chan));
if (chan->chan_qfreeze != 0) {
mutex_exit(chan_mtx(chan));
break;
}
...
}
And when scsipi_channel_thaw brings the freezecnt down to zero, it
does scsipi_run_queue.
So freezing the channel should have the same effect as
scsipi_done_once, and then thawing the channel should have the same
effect as scsipi_run_queue -- unless there is something else
interesting going on like a recursion of scsipi_done into itself in a
surprising place.
