RE: 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]

 





-----Original Message-----
From: Finn Thain [mailto:fthain@xxxxxxxxxxxxxxxxxxx]
Sent: Friday, February 12, 2021 1:09 PM
To: Song Bao Hua (Barry Song) <song.bao.hua@xxxxxxxxxxxxx>
Cc: tanxiaofei <tanxiaofei@xxxxxxxxxx>; jejb@xxxxxxxxxxxxx;
martin.petersen@xxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx;
linux-kernel@xxxxxxxxxxxxxxx; linuxarm@xxxxxxxxxxxxx;
linux-m68k@xxxxxxxxxxxxxxx
Subject: RE: Re: [PATCH for-next 00/32] spin lock usage optimization for SCSI
drivers

On Fri, 12 Feb 2021, Song Bao Hua (Barry Song) wrote:


-----Original Message-----
From: Finn Thain [mailto:fthain@xxxxxxxxxxxxxxxxxxx]
Sent: Friday, February 12, 2021 12:57 PM
To: Song Bao Hua (Barry Song) <song.bao.hua@xxxxxxxxxxxxx>
Cc: tanxiaofei <tanxiaofei@xxxxxxxxxx>; jejb@xxxxxxxxxxxxx;
martin.petersen@xxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx;
linux-kernel@xxxxxxxxxxxxxxx; linuxarm@xxxxxxxxxxxxx;
linux-m68k@xxxxxxxxxxxxxxx
Subject: RE: Re: [PATCH for-next 00/32] spin lock usage optimization for
SCSI
drivers


On Thu, 11 Feb 2021, Song Bao Hua (Barry Song) wrote:


Actually in m68k, I also saw its IRQ entry disabled interrupts by
' move	#0x2700,%sr		/* disable intrs */'

arch/m68k/include/asm/entry.h:

.macro SAVE_ALL_SYS
	move	#0x2700,%sr		/* disable intrs */
	btst	#5,%sp@(2)		/* from user? */
	bnes	6f			/* no, skip */
	movel	%sp,sw_usp		/* save user sp */
...

.macro SAVE_ALL_INT
	SAVE_ALL_SYS
	moveq	#-1,%d0			/* not system call entry */
	movel	%d0,%sp@(PT_OFF_ORIG_D0)
.endm

arch/m68k/kernel/entry.S:

/* This is the main interrupt handler for autovector interrupts */

ENTRY(auto_inthandler)
	SAVE_ALL_INT
	GET_CURRENT(%d0)
					|  put exception # in d0
	bfextu	%sp@(PT_OFF_FORMATVEC){#4,#10},%d0
	subw	#VEC_SPUR,%d0

	movel	%sp,%sp@-
	movel	%d0,%sp@-		|  put vector # on stack
auto_irqhandler_fixup = . + 2
	jsr	do_IRQ			|  process the IRQ
	addql	#8,%sp			|  pop parameters off stack
	jra	ret_from_exception

So my question is that " move	#0x2700,%sr" is actually disabling
all interrupts? And is m68k actually running irq handlers
with interrupts disabled?


When sonic_interrupt() executes, the IPL is 2 or 3 (since either IRQ may
be involved). That is, SR & 0x700 is 0x200 or 0x300. The level 3 interrupt
may interrupt execution of the level 2 handler so an irq lock is used to
avoid re-entrance.

This patch,

diff --git a/drivers/net/ethernet/natsemi/sonic.c
b/drivers/net/ethernet/natsemi/sonic.c
index d17d1b4f2585..041354647bad 100644
--- a/drivers/net/ethernet/natsemi/sonic.c
+++ b/drivers/net/ethernet/natsemi/sonic.c
@@ -355,6 +355,8 @@ static irqreturn_t sonic_interrupt(int irq, void *dev_id)
         */
        spin_lock_irqsave(&lp->lock, flags);

+       printk_once(KERN_INFO "%s: %08lx\n", __func__, flags);
+
        status = SONIC_READ(SONIC_ISR) & SONIC_IMR_DEFAULT;
        if (!status) {
                spin_unlock_irqrestore(&lp->lock, flags);

produces this output,

[    3.800000] sonic_interrupt: 00002300

I actually hope you can directly read the register rather than reading
a flag which might be a software one not from register.


Again, the implementation of arch_local_irq_save() may be found in
arch/m68k/include/asm/irqflags.h

Yes. I have read it. Anyway, I started a discussion in genirq
with you cc-ed:
https://lore.kernel.org/lkml/c46ddb954cfe45d9849c911271d7ec23@xxxxxxxxxxxxx/

And thanks very much for all your efforts to help me understand
M68k. Let's get this clarified thoroughly in genirq level.

In arm, we also have some special high-priority interrupts
which are not NMI but able to preempt normal IRQ. They are
managed by arch-extended APIs rather than common APIs.

Neither arch_irqs_disabled() nor local_irq_disable() API can
access this kind of interrupts. They are using things specific
to ARM like:
local_fiq_disable()
local_fiq_enable()
set_fiq_handler()
disable_fiq()
enable_fiq()
...

so fiq doesn't bother us anyhow in genirq.



I ran that code in QEMU, but experience shows that Apple hardware works
exactly the same. Please do confirm this for yourself, if you still think
the code and comments in sonic_interrupt are wrong.

Best Regards
Barry



Thanks
Barry




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

  Powered by Linux