> On Tue, May 22, 2001 at 03:18:36PM -0700, Matthew Jacob wrote:
> > 
> > > I think I can make scsipi_channel_thaw() run the queue if the count drops
> > > to 0. This may require changes to other drivers using
> > > scsipi_channel_thaw() to make sure it's safe to queue commands again when
> > > scsipi_channel_thaw() is used, but this should be easy.
> > > 
> > > I'll try to look at this tomorow.
> > 
> > Okay, thanks.
> > 
> > Insofar as keeping a count- this one can do, but it makes the added value
> > of this midlayer drop. It should be clear who can and who cannot modify the
> > channel freeze count, and when- otherwise I can guarantee you that there will
> > be cases where nobody will unfreeze when they should. I'd argue that the
> > channel freeze count should be read-only for the midlayer (except by being
> > tweaked via the freeze/thaw functions) and solely up to the HBA as to what
> > value it should be.
> 
> No, the channel freeze count can be read by the HBA driver (although maybe we
> should provide a macro for this too, if we ever have lock issues with SMP),
> but it should only be modified by scsipi_channel_freeze/thaw.
> For example, you may provide an ioctl to freeze a channel from userland;
> you don't want to have someone unlock it in your back. This is not
> only for the HBA use.

That was my question/point- it really should be one or the other. The decision
is "the midlayer".

> Also I think that the commit you did about negative freeze count should go
> away. When something freeze a channel, the same thing should thaw it, with
> the same count.

I don't agree. Theoretically, things should be paired. However, in case of
mistakes, I still prefer my system to run.

> For example, in your case you want to freeze the channel only one time, but
> from different places. So you just don't want to freeze a channel again if
> it's already frozen by the HBA driver. Keep a flag in the HBA driver, test
> it to know if it's already frozen. This way the freeze count of the HBA
> driver will always be 0 or 1, and you need to thaw it only with a count of 1.

Well, I don't much agree with all of this. It's not very practial, IMO. If you
have to keep a flag or a count in the HBA driver, you might as well queue in
the HBA driver.

> As for calling scsipi_run_queue() from scsipi_channel_thaw(),
> I still don't know what's best: 
> - have scsipi_channel_thaw() wake up the thread to run the queue. This may
>   help keeping commands sorted. This has also the side effect that if
>   scsipi_channel_thaw() is called from the HBA's interrupt routine,
>   scsipi_run_queue() will be delayed until the interrupt routine returns.
>   But this may not be true any more in SMP context.
> - Call scsipi_run_queue() directly from scsipi_channel_thaw(). Then adapters
>   drivers needs to be carefull when this is called in order to preserve
>   commands ordering.
> 
> I prefer the second solution, because it's more SMP-safe WRT commands ordering.
> HBA drivers writers have to DTRT now :)
> 

Well, supposedly you've sent all commands back to be requeued already with 
calls to scsipi_done (XS_REQUEUE).


I'll merge in your latest commits, and if I can get things running as I need
to, that's fine. If not, I'll bring the subject up again.

-matt