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

[Finn Thain]

On Tue, 9 Feb 2021, Song Bao Hua (Barry Song) wrote:

> > -----Original Message-----
> > From: Finn Thain [mailto:fthain@xxxxxxxxxxxxxxxxxxx]
> > Sent: Monday, February 8, 2021 8:57 PM
> > To: tanxiaofei <tanxiaofei@xxxxxxxxxx>
> > Cc: jejb@xxxxxxxxxxxxx; martin.petersen@xxxxxxxxxx;
> > linux-scsi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > linuxarm@xxxxxxxxxxxxx
> > Subject: [Linuxarm] Re: [PATCH for-next 00/32] spin lock usage optimization
> > for SCSI drivers
> > 
> > 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.