>
> On 08/31/2010 06:56 PM, Nicholas A. Bellinger wrote:
> >> + if (host->unlocked_qcmds)
> >> + spin_unlock_irqrestore(host->host_lock, flags);
> >> +
> >> if (unlikely(host->shost_state == SHOST_DEL)) {
> >> cmd->result = (DID_NO_CONNECT<< 16);
> >> scsi_done(cmd);
> >
> > I don't think it's safe to call scsi_done() for the SHOST_DEL case here
> > with host->unlocked_qcmds=1 w/o holding host_lock, nor would it be safe
> > for the SCSI LLD itself using host->unlocked_qcmds=1 to call the
> > (*scsi_done)() being passed into sht->queuecommand() without
> > host->host_lock being held by either the SCSI ML or the SCSI LLD.
>
> The host state should be checked under the host lock, but I do not think
> it needs to be held with calling scsi_done. scsi_done just queues up the
> request to be completed in the block softirq, and the block layer
> protects against something like the command getting completed by
> multiple code paths/threads.
It looks safe to me to call scsi_done() w/o host_lock held, in which case,
there is probably no need for the flag unlocked_qcmds, but just move the
spin_unlock_ireqrestore() up to just after scsi_cmd_get_serial(), and let
queuecommand() decide when/where if it wants to grab&drop the host lock, where
in the case for fc_queuecomamnd(), we won't grab it at all. Just a thought...
Thanks,
yi
--
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
