"Justin T. Gibbs" <gibbs@freefall.freebsd.org> writes:

> 
> >1) What is the point of passing down the scsi_queue to the *_get_ccb()
> >and *_free_ccb() routines?  What would they use it for?
> 
> A single controller could be in charge of several different queues.  This
> is geared toward multi-channel/RAID devices.  It is true that its not
> used for anything yet.
> 
> >2) Why isn't the scsi_queue allocated by the bus, rather than by the
> >controller (i.e. by scsi_alloc_bus(), in your configuration `model')?
> >(Presumably the function pointers would be passed down in the *_switch
> >structure.)
> 
> Because some controllers share a queue across mutliple busses.  The 2742T
> is one of these.

Uh, so?  In my little world, *all* of this knowledge would be in the
controller driver.  I'd say `Please make sure you have resources for N
requests available.', and the driver would tell me `Okay; I have
resources for M requests.  Shoot.'  The controller driver would
internally contain all the knowledge about where these are, and
whether or not they're shared between channels.  There's no reason to
expose any of this outside.

> >4) Related to the previous, under precisely what conditions is the
> >*_scsi_cmd() routine *not* supposed to call scsi_xs_started()?  Are
> >there any?  (If not, why isn't that call done before even calling
> >*_scsi_cmd()?  If you need a lock on the target, it should be
> >separate, and should either be done explicitly from *_scsi_cmd()
> >*after the CDB is actually sent to the adapter* (or at least
> >reserved), or just automatically when the routine finishes.  This is
> >not currently the case.)
> 
> scsi_xs_started should always be called as soon as the controller can
> guarantee that the queuing order of the xs cannot be perterbed by an
> interrupt or other event.

I know what it's for.  That wasn't the point.

> This is a case of not being able to have your cake and eat it too.  I
> wanted to centralize the handling of errors to the scsi_done routine
> but I couldn't prevent deadlock unless the lock was released by the
> controller at the proper time since there are cases when scsi_done is
> called before the *_scsi_cmd call completes.  I suppose I could record
> the xs that holds the lock on the target, but this seemed a little excessive.
> The current approach is actually more efficient anyway since the target is
> only locked as long as it absolutely has to be.

Where do you see a deadlock?  If there was an error, the command might
get tossed back onto the queue.  In any case, since you're already in
the process of dispatching commands, you shouldn't dispatch any from
the error handler, so the lock shouldn't even be touched.  (If you
*did* dispatch commands from the error handler in this case, you could
recurse indefinitely and overflow your stack!)

> >8) The use of `_scsi_flags' to deal with autoconfiguration is highly
> >questionable.  The old method of passing around the flags worked, and
> >didn't create a hidden interface.
> 
> This is because you don't know of my hidden agenda.  I'd like to see polling
> go away entirely or at least only happen for dumps.

This an *entirely* separate issue from the way queueing is handled,
and should be addressed at a later time.

> >9) It's unclear to me that actually keeping a pointer to the
> >controller-specific data in the xs is correct.  As I proposed this
> >change, the extra calldowns would merely reserve space on the
> >controller.  I believe this is better because it doesn't put *any*
> >knowledge of how the controller works internally into the xs.
> 
> I won't even touch on who proposed what...  Anyway, the XS has no
> knowlege of what the controller put in the cdb field.

But it knows that the controller *has* such a piece of memory, and
that the pointer to it is representable in a `void *'.  Why should it
even know this much, when it's completely trivial to have it not do
so?

> Why complicate
> the controller driver's job by forcing it to keep track of what is
> reserved and what is not when this strategy is so simple and offers
> a tight association of the resources.

Complicate its job by keeping track of how many it has free, rather
than exposing knowledge that it shouldn't have, and forcing me to
reserve one at a time?  My method is actually faster, in addition to
being more architecturally sound.