From: "Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com>
To: Finn Thain <fthain@telegraphics.com.au>
Cc: tanxiaofei <tanxiaofei@huawei.com>,
"jejb@linux.ibm.com" <jejb@linux.ibm.com>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linuxarm@openeuler.org" <linuxarm@openeuler.org>,
"linux-m68k@vger.kernel.org" <linux-m68k@vger.kernel.org>
Subject: RE: [Linuxarm] Re: [PATCH for-next 00/32] spin lock usage optimization for SCSI drivers
Date: Wed, 10 Feb 2021 23:49:10 +0000 [thread overview]
Message-ID: <3ec7cb32aa754a59b894d048873132cf@hisilicon.com> (raw)
In-Reply-To: <221cb29-53a8-fd1-4232-360655f28f3@telegraphics.com.au>
> -----Original Message-----
> From: Finn Thain [mailto:fthain@telegraphics.com.au]
> Sent: Thursday, February 11, 2021 11:35 AM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>
> Cc: tanxiaofei <tanxiaofei@huawei.com>; jejb@linux.ibm.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org;
> linux-kernel@vger.kernel.org; linuxarm@openeuler.org;
> linux-m68k@vger.kernel.org
> Subject: RE: [Linuxarm] Re: [PATCH for-next 00/32] spin lock usage optimization
> for SCSI drivers
>
> On Wed, 10 Feb 2021, Song Bao Hua (Barry Song) wrote:
>
> > > On Wed, 10 Feb 2021, Song Bao Hua (Barry Song) wrote:
> > >
> > > > >
> > > > > There is no warning from m68k builds. That's because
> > > > > arch_irqs_disabled() returns true when the IPL is non-zero.
> > > >
> > > > So for m68k, the case is
> > > > arch_irqs_disabled() is true, but interrupts can still come?
> > > >
> > > > Then it seems it is very confusing. If prioritized interrupts can
> > > > still come while arch_irqs_disabled() is true,
> > >
> > > Yes, on m68k CPUs, an IRQ having a priority level higher than the
> > > present priority mask will get serviced.
> > >
> > > Non-Maskable Interrupt (NMI) is not subject to this rule and gets
> > > serviced regardless.
> > >
> > > > how could spin_lock_irqsave() block the prioritized interrupts?
> > >
> > > It raises the the mask level to 7. Again, please see
> > > arch/m68k/include/asm/irqflags.h
> >
> > Hi Finn,
> > Thanks for your explanation again.
> >
> > TBH, that is why m68k is so confusing. irqs_disabled() on m68k should
> > just reflect the status of all interrupts have been disabled except NMI.
> >
> > irqs_disabled() should be consistent with the calling of APIs such as
> > local_irq_disable, local_irq_save, spin_lock_irqsave etc.
> >
>
> When irqs_disabled() returns true, we cannot infer that
> arch_local_irq_disable() was called. But I have not yet found driver code
> or core kernel code attempting that inference.
>
> > >
> > > > Isn't arch_irqs_disabled() a status reflection of irq disable API?
> > > >
> > >
> > > Why not?
> >
> > If so, arch_irqs_disabled() should mean all interrupts have been masked
> > except NMI as NMI is unmaskable.
> >
>
> Can you support that claim with a reference to core kernel code or
> documentation? (If some arch code agrees with you, that's neither here nor
> there.)
I think those links I share you have supported this. Just you don't
believe :-)
>
> > >
> > > Are all interrupts (including NMI) masked whenever
> > > arch_irqs_disabled() returns true on your platforms?
> >
> > On my platform, once irqs_disabled() is true, all interrupts are masked
> > except NMI. NMI just ignore spin_lock_irqsave or local_irq_disable.
> >
> > On ARM64, we also have high-priority interrupts, but they are running as
> > PESUDO_NMI:
> > https://lwn.net/Articles/755906/
> >
>
> A glance at the ARM GIC specification suggests that your hardware works
> much like 68000 hardware.
>
> When enabled, a CPU interface takes the highest priority pending
> interrupt for its connected processor and determines whether the
> interrupt has sufficient priority for it to signal the interrupt
> request to the processor. [...]
>
> When the processor acknowledges the interrupt at the CPU interface, the
> Distributor changes the status of the interrupt from pending to either
> active, or active and pending. At this point the CPU interface can
> signal another interrupt to the processor, to preempt interrupts that
> are active on the processor. If there is no pending interrupt with
> sufficient priority for signaling to the processor, the interface
> deasserts the interrupt request signal to the processor.
>
> https://developer.arm.com/documentation/ihi0048/b/
>
> Have you considered that Linux/arm might benefit if it could fully exploit
> hardware features already available, such as the interrupt priority
> masking feature in the GIC in existing arm systems?
I guess no:-) there are only two levels: IRQ and NMI. Injecting a high-prio
IRQ level between them makes no sense.
To me, arm64's design is quite clear and has no any confusion.
>
> > On m68k, it seems you mean:
> > irq_disabled() is true, but high-priority interrupts can still come;
> > local_irq_disable() can disable high-priority interrupts, and at that
> > time, irq_disabled() is also true.
> >
> > TBH, this is wrong and confusing on m68k.
> >
>
> Like you, I was surprised when I learned about it. But that doesn't mean
> it's wrong. The fact that it works should tell you something.
>
The fact is that m68k lets arch_irq_disabled() return true to pretend
all IRQs are disabled while high-priority IRQ is still open, thus "pass"
all sanitizing check in genirq and kernel core.
> Things could always be made simpler. But discarding features isn't
> necessarily an improvement.
This feature could be used by calling local_irq_enable_in_hardirq()
in those IRQ handlers who hope high-priority interrupts to preempt it
for a while.
It shouldn't hide somewhere and make confusion.
On the other hand, those who care about realtime should use threaded
IRQ and let IRQ threads preempt each other.
Thanks
Barry
next prev parent reply other threads:[~2021-02-10 23:50 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-07 11:36 [PATCH for-next 00/32] spin lock usage optimization for SCSI drivers Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 01/32] scsi: 53c700: Replace spin_lock_irqsave with spin_lock in hard IRQ Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 02/32] scsi: ipr: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 03/32] scsi: lpfc: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 04/32] scsi: qla4xxx: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 05/32] scsi: BusLogic: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 06/32] scsi: a100u2w: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 07/32] scsi: a2091: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 08/32] scsi: a3000: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 09/32] scsi: aha1740: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 10/32] scsi: bfa: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 11/32] scsi: esp_scsi: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 12/32] scsi: gvp11: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 13/32] scsi: hptiop: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 14/32] scsi: ibmvscsi: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 15/32] scsi: initio: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 16/32] scsi: megaraid: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 17/32] scsi: mac53c94: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 18/32] scsi: mesh: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 19/32] scsi: mvumi: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 20/32] scsi: myrb: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 21/32] scsi: myrs: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 22/32] scsi: ncr53c8xx: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 23/32] scsi: nsp32: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 24/32] scsi: pmcraid: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 25/32] scsi: pcmcia: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 26/32] scsi: qlogicfas408: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 27/32] scsi: qlogicpti: " Xiaofei Tan
2021-02-07 11:36 ` [PATCH for-next 28/32] scsi: sgiwd93: " Xiaofei Tan
2021-02-07 11:37 ` [PATCH for-next 29/32] scsi: stex: " Xiaofei Tan
2021-02-07 11:37 ` [PATCH for-next 30/32] scsi: vmw_pvscsi: " Xiaofei Tan
2021-02-07 11:37 ` [PATCH for-next 31/32] scsi: wd719x: " Xiaofei Tan
2021-02-07 11:37 ` [PATCH for-next 32/32] scsi: advansys: " Xiaofei Tan
2021-02-08 7:57 ` [PATCH for-next 00/32] spin lock usage optimization for SCSI drivers Finn Thain
2021-02-09 1:48 ` [Linuxarm] " Song Bao Hua (Barry Song)
2021-02-09 5:06 ` Finn Thain
2021-02-09 5:33 ` Song Bao Hua (Barry Song)
2021-02-10 0:28 ` Finn Thain
2021-02-10 0:37 ` Song Bao Hua (Barry Song)
2021-02-10 4:14 ` Finn Thain
2021-02-09 5:46 ` Song Bao Hua (Barry Song)
2021-02-10 4:16 ` Finn Thain
2021-02-10 5:14 ` Song Bao Hua (Barry Song)
2021-02-10 21:06 ` Finn Thain
2021-02-10 21:28 ` Song Bao Hua (Barry Song)
2021-02-10 22:34 ` Finn Thain
2021-02-10 23:49 ` Song Bao Hua (Barry Song) [this message]
2021-02-11 1:11 ` Finn Thain
2021-02-11 3:02 ` Song Bao Hua (Barry Song)
2021-02-11 23:58 ` Finn Thain
2021-02-12 0:21 ` Song Bao Hua (Barry Song)
2021-02-18 7:12 ` Xiaofei Tan
2021-02-20 5:18 ` Finn Thain
2021-02-22 2:04 ` Song Bao Hua (Barry Song)
2021-02-23 5:25 ` Finn Thain
2021-02-23 5:47 ` Song Bao Hua (Barry Song)
2021-02-24 5:20 ` Finn Thain
2021-02-24 10:50 ` Song Bao Hua (Barry Song)
2021-02-25 7:07 ` Finn Thain
2021-02-09 2:00 ` tanxiaofei
2021-02-09 5:11 ` Finn Thain
2021-02-24 9:41 ` Geert Uytterhoeven
2021-02-25 2:37 ` [Linuxarm] " Xiaofei Tan
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=3ec7cb32aa754a59b894d048873132cf@hisilicon.com \
--to=song.bao.hua@hisilicon.com \
--cc=fthain@telegraphics.com.au \
--cc=jejb@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=linuxarm@openeuler.org \
--cc=martin.petersen@oracle.com \
--cc=tanxiaofei@huawei.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;
as well as URLs for NNTP newsgroup(s).