On Thu, 2010-09-16 at 14:44 -0700, Nicholas A. Bellinger wrote:
> On Thu, 2010-09-16 at 14:34 -0700, Nicholas A. Bellinger wrote:
> > On Thu, 2010-09-16 at 23:25 +0200, Andi Kleen wrote:
> > > > I asked James about getting Vasu's unlocked_qcmds=1 patch merged, but he
> > > > convinced me that doing conditional locking while is very simple, is not
> > > > the proper way for getting this resolved in mainline code. I think in
> > > > the end this will require a longer sit down to do a wholesale conversion
> > > > of all existing SCSI LLD drivers, and identifing the broken ones that
> > > > still need a struct Scsi_Host->host_lock'ed SHT->queuecommand() for
> > > > whatever strange & legacy reasons.
> > >
> > > The standard way to do that would be to first move the lock down
> > > into the drivers (similar to how it has been done with the BKL).
> > > This would be a fairly mechanic mindless patch. Lots of typing,
> > > but not really a lot of real code review needed.
> > >
> > > Then next step the drivers who know they don't want it can remove it.
> >
> > Well, the main bit is finding those drives that actually call some form
> > of struct Scsi_Host->host_lock unlock() -> do_work() -> lock() from
> > within their own ->queuecomamnd() caller, and avoid running into the big
> > ugly deadlock that would happen here..
> >
> > I know that Open-iSCSi and my own iSCSI Initiator do this
> > "optimization", but I am not sure which other LLDs also do this as
> > well..
> >
> > Best,
> >
>
> Ok, spotted another one of these in drivers/scsi/lpfc/lpfc_scsi.c:lpfc_queuecommand():
>
> <SNIP>
>
> if (phba->cfg_poll & ENABLE_FCP_RING_POLLING) {
> spin_unlock(shost->host_lock);
> lpfc_sli_handle_fast_ring_event(phba,
> &phba->sli.ring[LPFC_FCP_RING], HA_R0RE_REQ);
>
> spin_lock(shost->host_lock);
> if (phba->cfg_poll & DISABLE_FCP_RING_INT)
> lpfc_poll_rearm_timer(phba);
> }
>
> return 0;
>
> Best,
And ditto ->queuecommand() with qla2xxx and qla4xxx LLDs as well:
<SNIP>
spin_unlock_irq(ha->host->host_lock);
srb = qla4xxx_get_new_srb(ha, ddb_entry, cmd, done);
if (!srb)
goto qc_host_busy_lock;
rval = qla4xxx_send_command_to_isp(ha, srb);
if (rval != QLA_SUCCESS)
goto qc_host_busy_free_sp;
spin_lock_irq(ha->host->host_lock);
return 0;
So yeah, it looks like the high performance LLDs have adopted this trick
over the years.. :-)
Best,
--nab
--
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
