Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2

Jeff Garzik
2004-09-14T18:12:54Z

Mark Lord wrote:
> >In particular, releasing the lock and sleeping would be very wrong
> >in the ->queuecommand and error handling paths
> >(alas... I would love to sleep in the fine-grained eh hooks)
>
> Mmm.. definitely no sleeps in queuecommand(), but sleeping seems
> necessary in host_reset_handler() -- the alternative is to just
> busywait inline.. which would really not be good.
>
> Isn't the protocol for the eh host_reset_handler() basically
> just "do the reset, and return whether it worked or not?".
> If so, the driver really has to hang around until the reset
> completes so that correct status can be returned. This generally
> takes a couple of milliseconds in practice (measured it).
>
> Is there a better way to do that?
>
> I really would prefer never to have to reset the drives,
> but when they have a queuing error, many of them simply
> won't talk to us again without a reset. The driver avoids
> the reset as much as it can for other situations, though.

James and I occasionally talk about this. This is a _big_ reason why I
use the ->eh_strategy_handler() rather than the more fine-grained ->eh
hooks: you get unlocked, unfettered sleep priveleges inside the scsi EH
thread.

The SCSI LLD API really needs to -not- spinlock on the EH hooks, and
instead simply guarantee that ->queuecommand and other hooks will not be
called while the driver is in EH.

ISTR James didn't disagree, so maybe a patch can be worked out...

Of course, you could always just use ->eh_strategy_handler and do 100%
of the error handling yourself. That's the route libata chose.

Jeff