All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: James Bottomley <James.Bottomley@suse.de>,
	Andi Kleen <ak@linux.intel.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Eric Moore <Eric.Moore@lsi.com>,
	linux-scsi@vger.kernel.org, vasu.dev@intel.com,
	willy@linux.intel.com
Subject: Re: [PATCH] scsi, mptsas : drop scsi_host lock when calling mptsas_qcmd
Date: Thu, 16 Sep 2010 17:13:45 -0700	[thread overview]
Message-ID: <1284682425.13344.130.camel@haakon2.linux-iscsi.org> (raw)
In-Reply-To: <20100916232506.GA19476@infradead.org>

On Thu, 2010-09-16 at 19:25 -0400, Christoph Hellwig wrote:
> On Thu, Sep 16, 2010 at 05:24:47PM -0400, James Bottomley wrote:
> > > 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.
> > 
> > Yes, that's basically what Christoph did when he moved the lock out of
> > the eh path.
> 
> And it is what we should do here as well.  I'm just wondering if we rely
> on the fact that we hold the lock over the check for the device beeing
> deleted and the queuecommand call.
> 

Ugh, I completely forgot the about the (host->shost_state == SHOST_DEL)
check in scsi_dispatch_cmd() in patch #1, and just fixed this in
lio-core-2.6.git/drop-host_lock here:

http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=commitdiff;h=e222907d9baac56425f87c602acdd757dc5fde08

Here is what the updated scsi_dispatch_cmd() now looks like:

	<SNIP>

        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
         */
        scsi_cmd_get_serial(host, cmd);

        if (unlikely(host->shost_state == SHOST_DEL)) {
                spin_unlock_irqrestore(host->host_lock, flags);
                cmd->result = (DID_NO_CONNECT << 16);
                scsi_done(cmd);
        } else {
                spin_unlock_irqrestore(host->host_lock, flags);
                trace_scsi_dispatch_cmd_start(cmd);
                rtn = host->hostt->queuecommand(cmd, scsi_done);
        }

	<SNIP>

Thanks!

--nab



  reply	other threads:[~2010-09-17  0:17 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-16 19:44 [PATCH] scsi, mptsas : drop scsi_host lock when calling mptsas_qcmd Tim Chen
2010-09-16 20:48 ` Nicholas A. Bellinger
2010-09-16 21:18   ` Tim Chen
2010-09-16 21:25   ` Andi Kleen
2010-09-16 21:24     ` James Bottomley
2010-09-16 23:25       ` Christoph Hellwig
2010-09-17  0:13         ` Nicholas A. Bellinger [this message]
2010-09-17  1:12           ` Vasu Dev
2010-09-16 21:34     ` Nicholas A. Bellinger
2010-09-16 21:44       ` Nicholas A. Bellinger
2010-09-16 21:48         ` Nicholas A. Bellinger
2010-09-16 22:00     ` Joe Eykholt
2010-09-16 22:16       ` James Bottomley
2010-09-17  7:16         ` Andi Kleen
2010-09-17 10:32           ` Bart Van Assche
2010-09-17 12:19             ` James Bottomley
2010-09-16 22:26       ` Tim Chen
2010-09-16 21:31   ` Vasu Dev

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=1284682425.13344.130.camel@haakon2.linux-iscsi.org \
    --to=nab@linux-iscsi.org \
    --cc=Eric.Moore@lsi.com \
    --cc=James.Bottomley@suse.de \
    --cc=ak@linux.intel.com \
    --cc=hch@infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=tim.c.chen@linux.intel.com \
    --cc=vasu.dev@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.