Re: [Linuxarm] Re: [PATCH for-next 00/32] spin lock usage optimization for SCSI drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 18 Feb 2021, Xiaofei Tan wrote:

On 2021/2/9 13:06, Finn Thain wrote:
On Tue, 9 Feb 2021, Song Bao Hua (Barry Song) wrote:

On Sun, 7 Feb 2021, Xiaofei Tan wrote:

Replace spin_lock_irqsave with spin_lock in hard IRQ of SCSI 
drivers. There are no function changes, but may speed up if 
interrupt happen too often.

This change doesn't necessarily work on platforms that support 
nested interrupts.

Were you able to measure any benefit from this change on some 
other platform?

I think the code disabling irq in hardIRQ is simply wrong.
Since this commit
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e58aa3d2d0cc
genirq: Run irq handlers with interrupts disabled

interrupt handlers are definitely running in a irq-disabled context
unless irq handlers enable them explicitly in the handler to permit
other interrupts.


Repeating the same claim does not somehow make it true. If you put 
your claim to the test, you'll see that that interrupts are not 
disabled on m68k when interrupt handlers execute.

The Interrupt Priority Level (IPL) can prevent any given irq handler 
from being re-entered, but an irq with a higher priority level may be 
handled during execution of a lower priority irq handler.

sonic_interrupt() uses an irq lock within an interrupt handler to 
avoid issues relating to this. This kind of locking may be needed in 
the drivers you are trying to patch. Or it might not. Apparently, 
no-one has looked.


According to your discussion with Barry, it seems that m68k is a little 
different from other architecture, and this kind of modification of this 
patch cannot be applied to m68k. So, could help to point out which 
driver belong to m68k architecture in this patch set of SCSI? I can 
remove them.


If you would claim that "there are no function changes" in your patches 
(as above) then the onus is on you to support that claim.

I assume that there are some platforms on which your assumptions hold.

With regard to drivers for those platforms, you might want to explain why 
your patches should be applied there, given that the existing code is 
superior for being more portable.

BTW, sonic_interrupt() is from net driver natsemi, right?  It would be 
appreciative if only discuss SCSI drivers in this patch set. thanks.


The 'net' subsystem does have some different requirements than the 'scsi' 
subsystem. But I don't see how that's relevant. Perhaps you can explain 
it. Thanks.



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux