Re: SCSI changes - PR58452 for review

On Wed, 7 Aug 2024 10:21:28 Taylor R Campbell wrote:


> Instead of introducing scsipi_done_once, why not just freeze and thaw
> the channel around scsipi_done?
> 

I tried the following and it resulted in a slew of error messages from the 
kernel resetting the scsi bus/ aborting.

+	scsipi_channel_freeze(&sc->sc_channel, 1);
 
  	/* Tell common SCSI code it is done. */
  	scsipi_done(xs);
   	sc->sc_state = NCR_IDLE;

	/* Now ncr5380_sched() may be called again. */

+	scsipi_channel_thaw(&sc->sc_channel, 1);

> Alternatively, is it critical for scsipi_done to happen before
> sc->sc_state = NCR_IDLE?  Could we just swap the order of scsipi_done
> and sc->sc_state = NCR_IDLE?
> 

I tried this also which resulted in similar error messages.

I guess in both cases another request is sent to the controller in 
scsipi_run_queue, called from scsipi_done which results in an inconsistient 
state.

scspi_done_once, and thaw with an argument of 0 to kick the queue as posted 
earlier works great (with a noticable improvent - most shell commands 
responding/starting twice as fast as usual).

> (I wonder why only this scsi driver seems to have the issue.  Is it
> doing something wonky with maintaining its own copy of some kind of
> driver state that is inconsistent with the normal control flow of the
> scsi framework?)

Not sure I don't know if other scsi controller drivers have this issue (cjep@ 
reported to me that there was kernel message "Should have flushed the queue" 
reported with bluescsi-v2 and esp(4) when used along with the dse(4) driver).