> 
> 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

_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel

Reply via email to