linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
To: Brian King <brking@linux.vnet.ibm.com>
Cc: linux-scsi <linux-scsi@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Vasu Dev <vasu.dev@linux.intel.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Andi Kleen <ak@linux.intel.com>,
	Matthew Wilcox <willy@linux.intel.com>,
	James Bottomley <James.Bottomley@suse.de>,
	Mike Christie <michaelc@cs.wisc.edu>,
	James Smart <james.smart@emulex.com>,
	Andrew Vasquez <andrew.vasquez@qlogic.com>,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>,
	Hannes Reinecke <hare@suse.de>, Joe Eykholt <jeykholt@cisco.com>,
	Christoph Hellwig <hch@lst.de>,
	MPTFusionLinux <DL-MPTFusionLinux@lsi.com>,
	"eata.c maintainer" <dario.ballabio@inwind.it>
Subject: Re: [RFC v3 01/15] scsi: Drop struct Scsi_Host->host_lock usage in scsi_dispatch_cmd()
Date: Fri, 24 Sep 2010 13:44:00 -0700	[thread overview]
Message-ID: <1285361040.1849.235.camel@haakon2.linux-iscsi.org> (raw)
In-Reply-To: <4C9CAA80.1030702@linux.vnet.ibm.com>

On Fri, 2010-09-24 at 08:41 -0500, Brian King wrote:
> On 09/23/2010 06:37 PM, Nicholas A. Bellinger wrote:
> > @@ -651,7 +655,6 @@ static inline void scsi_cmd_get_serial(struct Scsi_Host *host, struct scsi_cmnd
> >  int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
> >  {
> >  	struct Scsi_Host *host = cmd->device->host;
> > -	unsigned long flags = 0;
> >  	unsigned long timeout;
> >  	int rtn = 0;
> > 
> > @@ -736,15 +739,11 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
> >  		scsi_done(cmd);
> >  		goto out;
> >  	}
> > -
> > -	spin_lock_irqsave(host->host_lock, flags);
> >  	/*
> > -	 * AK: unlikely race here: for some reason the timer could
> > -	 * expire before the serial number is set up below.
> > -	 *
> > -	 * TODO: kill serial or move to blk layer
> > +	 * Note that scsi_cmd_get_serial() used to be called here, but
> > +	 * now we expect the legacy SCSI LLDs that actually need this
> > +	 * to call it directly within their SHT->queuecommand() caller.
> >  	 */
> > -	scsi_cmd_get_serial(host, cmd); 
> > 
> >  	if (unlikely(host->shost_state == SHOST_DEL)) {
> >  		cmd->result = (DID_NO_CONNECT << 16);
> > @@ -753,7 +752,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
> >  		trace_scsi_dispatch_cmd_start(cmd);
> >  		rtn = host->hostt->queuecommand(cmd, scsi_done);
> >  	}
> > -	spin_unlock_irqrestore(host->host_lock, flags);
> > +
> >  	if (rtn) {
> >  		trace_scsi_dispatch_cmd_error(cmd, rtn);
> >  		if (rtn != SCSI_MLQUEUE_DEVICE_BUSY &&
> 
> Are you planning a future revision that moves the acquiring of the host lock
> into the LLDD's queuecommand for all the other drivers you don't currently
> touch in this patch set?
> 

Hi Brian,

I was under the impression that this would be unnecessary for the vast
majority of existing LLD drivers, but if you are aware of specific LLDs
that would still need the struct Scsi_Host->host_lock held in their
SHT->queuecommand() for whaterver reason please let me know and I would
be happy to include this into an RFCv4.

Thanks for your comments!

--nab

  reply	other threads:[~2010-09-24 20:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-23 23:37 [RFC v3 01/15] scsi: Drop struct Scsi_Host->host_lock usage in scsi_dispatch_cmd() Nicholas A. Bellinger
2010-09-24 13:41 ` Brian King
2010-09-24 20:44   ` Nicholas A. Bellinger [this message]
2010-09-24 21:10     ` Brian King
2010-09-24 21:12       ` Nicholas A. Bellinger
2010-09-27 14:19     ` Christof Schmitt
2010-09-27 23:15       ` Nicholas A. Bellinger
  -- strict thread matches above, loose matches on Subject: below --
2010-09-24  0:46 Nicholas A. Bellinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1285361040.1849.235.camel@haakon2.linux-iscsi.org \
    --to=nab@linux-iscsi.org \
    --cc=DL-MPTFusionLinux@lsi.com \
    --cc=James.Bottomley@suse.de \
    --cc=ak@linux.intel.com \
    --cc=andrew.vasquez@qlogic.com \
    --cc=brking@linux.vnet.ibm.com \
    --cc=dario.ballabio@inwind.it \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=james.smart@emulex.com \
    --cc=jeykholt@cisco.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=tim.c.chen@linux.intel.com \
    --cc=vasu.dev@linux.intel.com \
    --cc=willy@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).