On Thu, 2010-10-28 at 10:09 +0200, Christof Schmitt wrote:
> On Thu, Oct 28, 2010 at 12:01:08AM -0700, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
> >
> > Following yet another round of convincing discussions from jgarzik and jejb
> > this afternoon, this RFCv8 patch series follows their recommendation and does a
> > SCSI subsystem wide 'push down' of struct Scsi_Host->host_lock usage from
> > scsi_dispatch_cmd() into legacy LLDs that require (or we are not sure about)
> > the host_lock held while SHT->queuecommand() I/O dispatch occurs.
> >
> > This patch still calls scsi_cmd_get_serial() for all LLDs within scsi_dispatch_cmd()
> > to perform struct scsi_cmnd->serial_number assignment using the new atomic_t
> > struct Scsi_Host->cmd_serial_number counter. This counter follows a recommedation by
> > Joe Eykholt to increment each serial_number by 2 so that the serial is odd, and wraps
> > to 1 instead of 0.
> >
> > Finally this patch also changes modern LLDs originally using the SHT->queuecommand() ->
> > unlock() -> do_some_lld_work() -> lock() -> return from lld_queuecommand()
> > optimization to now run by default in the 'host_lock-less' mode. This also
> > includes a handful of other LLDs that folks have tested in unlocked mode and
> > appear to be functioning as expected.
> [...]
> > diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
> > index 50286d8..0ca1bd3 100644
> > --- a/drivers/s390/scsi/zfcp_scsi.c
> > +++ b/drivers/s390/scsi/zfcp_scsi.c
> > @@ -82,8 +82,11 @@ static int zfcp_scsi_queuecommand(struct scsi_cmnd *scpnt,
> > struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
> > struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
> > struct fc_rport *rport = starget_to_rport(scsi_target(scpnt->device));
> > + unsigned long hflags;
> > int status, scsi_result, ret;
> >
> > + spin_lock_irqsave(scpnt->device->host->host_lock, hflags);
> > +
> > /* reset the status for this request */
> > scpnt->result = 0;
> > scpnt->host_scribble = NULL;
> > @@ -94,6 +97,7 @@ static int zfcp_scsi_queuecommand(struct scsi_cmnd *scpnt,
> > scpnt->result = scsi_result;
> > zfcp_dbf_scsi_fail_send(adapter->dbf, scpnt);
> > scpnt->scsi_done(scpnt);
> > + spin_unlock_irqrestore(scpnt->device->host->host_lock, hflags);
> > return 0;
> > }
> >
> > @@ -104,6 +108,7 @@ static int zfcp_scsi_queuecommand(struct scsi_cmnd *scpnt,
> > /* only LUN access denied, but port is good
> > * not covered by FC transport, have to fail here */
> > zfcp_scsi_command_fail(scpnt, DID_ERROR);
> > + spin_unlock_irqrestore(scpnt->device->host->host_lock, hflags);
> > return 0;
> > }
> >
> > @@ -115,15 +120,20 @@ static int zfcp_scsi_queuecommand(struct scsi_cmnd *scpnt,
> > * fc_remote_port_chkready until rport is BLOCKED
> > */
> > zfcp_scsi_command_fail(scpnt, DID_IMM_RETRY);
> > + spin_unlock_irqrestore(scpnt->device->host->host_lock, hflags);
> > return 0;
> > }
> >
> > ret = zfcp_fsf_fcp_cmnd(scpnt);
> > - if (unlikely(ret == -EBUSY))
> > + if (unlikely(ret == -EBUSY)) {
> > + spin_unlock_irqrestore(scpnt->device->host->host_lock, hflags);
> > return SCSI_MLQUEUE_DEVICE_BUSY;
> > - else if (unlikely(ret < 0))
> > + } else if (unlikely(ret < 0)) {
> > + spin_unlock_irqrestore(scpnt->device->host->host_lock, hflags);
> > return SCSI_MLQUEUE_HOST_BUSY;
> > + }
> >
> > + spin_unlock_irqrestore(scpnt->device->host->host_lock, hflags);
> > return ret;
> > }
> >
>
> For the zfcp part:
> Acked-by: Christof Schmitt <christof.schmitt@xxxxxxxxxx>
>
Added to forthcoming lio-4.0.git/host_lock-less-for-37-v9
> What is now the strategy for fc_remote_port_chkready? This function
> requires the host_lock to be held for protecting the
> rport->port_state. I am working on a patch to reduce the time where
> the host_lock is being held in the zfcp driver. But it looks like the
> FC LLDs have to keep something like this for now:
>
> spin_lock_irqsave(scpnt->device->host->host_lock, hflags);
> scsi_result = fc_remote_port_chkready(rport);
> spin_unlock_irqrestore(scpnt->device->host->host_lock, hflags);
>
> Are there any plans to change this for the rport->port_state?
>
This was an item that Mike Christie had originally mentioned, with the
current status being we are 'not sure' and awaiting a deeper review from
James Smart, Joe Eykholt, Robert Love, or someone else fimilar with the
libfc -> fcoe usage of rport w/ the legacy host_lock held.
So libfc folks, please let me know if what needs to be changed in order
to enable host_lock-less operation for libfc -> fcoe code for this round
of if we will need to defer for .38.
Thank you for your comments Christof!
--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
