On Fri, Nov 12, 2010 at 8:28 PM, Matthew Wilcox <matthew@xxxxxx> wrote:
> Actually, it'd be even better if we took away an argument.
>
> int queuecommand(struct scsi_cmnd *cmd);
>
> Every single driver sets cmd->scsi_done = done;
.. and i that's all they really ever do with it, I guess that that
would indeed be an improvement.
I still do think that passing in shost is actually a good idea, since
you'll normally be needing it for locking and other things anyway
(looking at the libata-scsi.c one, for example, the very first thing
it wants to do is to look up the port from the shost pointer).
Having to inevitably do that silly double dereference
("cmd->device->host") seems bogus. Passing in the host you want to
queue something on seems to be natural, and avoids that nasty chain of
dereferences, so it really should improve the code.
I don't think there are any drivers that don't want that host pointer
pretty much immediately, even if they can elide the locking. But I
admit I only looked at two (qla4xxx driver does the exact same thing:
the very first thing in the queuecommand function is
"to_qla_host(cmd->device->host)", but it wants that hostdata thing
too, so whatever.)
So the shost pointer is certainly a lot more useful than the 'done'
function pointer. So yeah, I'd switch them around - pass the 'done'
thing indirectly, and the shost directly.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
