public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jeff@garzik.org>
To: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
Cc: Boaz Harrosh <bharrosh@panasas.com>,
	Christoph Hellwig <hch@infradead.org>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	James Bottomley <James.Bottomley@suse.de>,
	Christoph Hellwig <hch@lst.de>,
	Mike Christie <michaelc@cs.wisc.edu>,
	Ravi Anand <ravi.anand@qlogic.com>,
	Andrew Vasquez <andrew.vasquez@qlogic.com>,
	Joe Eykholt <jeykholt@cisco.com>,
	James Smart <james.smart@emulex.com>,
	Vasu Dev <vasu.dev@linux.intel.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Andi Kleen <ak@linux.intel.com>, Tejun Heo <tj@kernel.org>,
	Mike Anderson <andmike@linux.vnet.ibm.com>,
	MPTFusionLinux <DL-MPTFusionLinux@lsi.com>
Subject: Re: [PATCH 03/11] scsi: Add IRQ_DISABLE_SCSI_QCMD wrapper
Date: Thu, 18 Nov 2010 18:15:00 -0500	[thread overview]
Message-ID: <4CE5B374.5050802@garzik.org> (raw)
In-Reply-To: <1290121510.31890.143.camel@haakon2.linux-iscsi.org>

On 11/18/2010 06:05 PM, Nicholas A. Bellinger wrote:
> On Thu, 2010-11-18 at 12:25 +0200, Boaz Harrosh wrote:
>> On 11/18/2010 12:37 AM, Christoph Hellwig wrote:
>>> On Wed, Nov 17, 2010 at 02:29:30PM -0800, Nicholas A. Bellinger wrote:
>>>> Hmmm, this is following jgarzik's recommendation for LLDs that we could
>>>> not immediately identify a internal spin_lock to disable interrupts
>>>> upon.  (eg: not libiscsi and libata).
>>>
>>> In that case wait for the driver author to identify it.  If there's
>>> no maintainer in reach chance is the driver doesn't care about the push
>>> down.  No need to rush any of this, do it one driver at a time and get
>>> it right.
>>
>> I totally agree with Christoph. Patches 5, 6, 8, 11 all change behaviour
>>
>> I would like to see an "I audit the driver and ..." Please see my comment
>> to [patch 6]
>
> Just to reiterate the point here..  The only LLDs that have been made
> 'lock-less' in this first round are LLDs that:
>
> *) Where already using the legacy optimization of releasing host_lock,
> doing lld_work, and reaquring before returning from their
> lld_queuecommand().
>
> *) Have been tested by folks explictly during the various initial
> attempts at lock_less mode, which still appear to be functionally
> equivilent minus the precatuion of local_irq_[save,release] usage.
>
>>
>> Do it one by one and open-code the local_irq_save/restore inside the
>> main function. It's not like you can get away from a total audit and
>> testing.
>>
>
> Hmmm, I am not sure I agree with open coding these atm.  I would much
> rather get LLD maintainer feedback first for these initial cases so we
> can figure out which of these patches can be further optimized along the
> lines of libiscsi and libata.

I don't think we'll be able to get around looking at each case 
individually, therefore I do not see the utility of adding 
IRQ_DISABLE_SCSI_QCMD().

If you're looking at each case, then you likely have an idea of exactly 
how the code should be updated.

	Jeff



  reply	other threads:[~2010-11-18 23:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-17 22:18 [PATCH 03/11] scsi: Add IRQ_DISABLE_SCSI_QCMD wrapper Nicholas A. Bellinger
2010-11-17 22:27 ` Christoph Hellwig
2010-11-17 22:29   ` Nicholas A. Bellinger
2010-11-17 22:37     ` Christoph Hellwig
2010-11-18 10:25       ` Boaz Harrosh
2010-11-18 23:05         ` Nicholas A. Bellinger
2010-11-18 23:15           ` Jeff Garzik [this message]
2010-11-19  0:04             ` Nicholas A. Bellinger
  -- strict thread matches above, loose matches on Subject: below --
2010-11-12  0:13 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=4CE5B374.5050802@garzik.org \
    --to=jeff@garzik.org \
    --cc=DL-MPTFusionLinux@lsi.com \
    --cc=James.Bottomley@suse.de \
    --cc=ak@linux.intel.com \
    --cc=andmike@linux.vnet.ibm.com \
    --cc=andrew.vasquez@qlogic.com \
    --cc=bharrosh@panasas.com \
    --cc=hch@infradead.org \
    --cc=hch@lst.de \
    --cc=james.smart@emulex.com \
    --cc=jeykholt@cisco.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=nab@linux-iscsi.org \
    --cc=ravi.anand@qlogic.com \
    --cc=tim.c.chen@linux.intel.com \
    --cc=tj@kernel.org \
    --cc=vasu.dev@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