Re: [PATCH 03/11] scsi: Add IRQ_DISABLE_SCSI_QCMD wrapper

Jeff Garzik
2010-11-18T23:15:05Z

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