On Thu, 2010-09-16 at 12:44 -0700, Tim Chen wrote: > During testing of FFSB benchmark (configured with > 128 threaded write to 128 files on 16 SSD), scsi_host lock was > heavily contended, accounting for 23.7% of cpu cycles. There > are 64 cores in our test system and the JBOD > is connected with a mptsas HBA. Taking a similar approach > as the patch by Vasu (http://permalink.gmane.org/gmane.linux.scsi.open-fcoe.devel/10110) > for Fiber Channel adapter, the following patch on 2.6.35 kernel > avoids taking the scsi host lock when queueing mptsas scsi command. We see > a big drop in the cpu cycles contending for the lock (from 23.7% to 1.8%). > The number of IO per sec increase by 10.6% from 62.9K per sec to 69.6K per sec. > > If there is no good reason to prevent mptsas_qcmd from being > executed in parallel, we should remove this lock from the queue > command code path. Other adapters probably can > benefit in a similar manner. > > > %cpu cycles contending host lock > 2.6.35 2.6.35+patch > ----------------------------------------------------- > scsi_dispatch_cmd 5.5% 0.44% > scsi_device_unbusy 6.1% 0.66% > scsi_request_fn 6.6% 0.35% > scsi_run_queue 5.5% 0.35% > > Hi Tim and Co, Many Thanks for posting these very interesting numbers with unlocked_qcmds=1 + mpt-fusion SCSI LLD on a 64-core system.. Wow.. 8-) 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. While there are still some outstanding TCM items that need to be resolved in the next days, I am very interested to help make the wholesale host_lock + ->queuecomamnd() conversion happen. I will get a lio-core-2.6.git branch setup for this purpose on .36-rc4 soon and start working on the main SCSI Mid-layer conversion pieces sometime next week. I am very eager to accept patches on a per LLD basis for this work, and will be starting with the open-fcoe initiator, TCM_Loop, mpt2sas, and open-iscsi. I think the wholesole conversion is going to be pretty straight-forward, and at least with the main SCSI LLDs (that we really care about ;) there appear to be no immediate issues with a full conversion. Best, --nab > Tim Chen > > > Signed-off-by: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> > diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c > index ac000e8..ab3aab9 100644 > --- a/drivers/message/fusion/mptsas.c > +++ b/drivers/message/fusion/mptsas.c > @@ -4984,6 +4984,8 @@ mptsas_probe(struct pci_dev *pdev, const struct pci_device_id *id) > sh->max_lun = max_lun; > sh->transportt = mptsas_transport_template; > > + sh->unlocked_qcmds = 1; > + > /* Required entry. > */ > sh->unique_id = ioc->id; > diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c > index ad0ed21..3819d66 100644 > --- a/drivers/scsi/scsi.c > +++ b/drivers/scsi/scsi.c > @@ -749,11 +749,16 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd) > if (unlikely(host->shost_state == SHOST_DEL)) { > cmd->result = (DID_NO_CONNECT << 16); > scsi_done(cmd); > + spin_unlock_irqrestore(host->host_lock, flags); > } else { > trace_scsi_dispatch_cmd_start(cmd); > + if (host->unlocked_qcmds) > + spin_unlock_irqrestore(host->host_lock, flags); > rtn = host->hostt->queuecommand(cmd, scsi_done); > + if (!host->unlocked_qcmds) > + spin_unlock_irqrestore(host->host_lock, flags); > } > - spin_unlock_irqrestore(host->host_lock, flags); > + > if (rtn) { > trace_scsi_dispatch_cmd_error(cmd, rtn); > if (rtn != SCSI_MLQUEUE_DEVICE_BUSY && > diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h > index b7bdecb..1814c51 100644 > --- a/include/scsi/scsi_host.h > +++ b/include/scsi/scsi_host.h > @@ -636,6 +636,9 @@ struct Scsi_Host { > /* Asynchronous scan in progress */ > unsigned async_scan:1; > > + /* call queuecommand without Scsi_Host lock held */ > + unsigned unlocked_qcmds:1; > + > /* > * Optional work queue to be utilized by the transport > */ > > > > -- > 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 -- 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
