Subject: Re: more new scsi midlayer questions
To: Manuel Bouyer <firstname.lastname@example.org>
From: Matthew Jacob <email@example.com>
Date: 05/23/2001 11:07:43
> 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.