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.
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.

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.

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 :)

--
Manuel Bouyer, LIP6, Universite Paris VI.           Manuel.Bouyer@lip6.fr
--