arch/arm64/include/asm/irqflags.h | 38 +++++++++++++++++++++++++++++-- arch/arm64/kernel/entry.S | 4 ++++ 2 files changed, 40 insertions(+), 2 deletions(-)
Our product has been updating its kernel from 4.4 to 5.10 recently and
found a performance issue. We do a bussiness test called ARP test, which
tests the latency for a ping-pong packets traffic with a certain payload.
The result is as following.
- 4.4 kernel: avg = ~20s
- 5.10 kernel (CONFIG_ARM64_PSEUDO_NMI is not set): avg = ~40s
I have been just learning arm64 pseudo-NMI code and have a question,
why is the related code not wrapped by CONFIG_ARM64_PSEUDO_NMI?
I wonder if this brings some performance regression.
First, I make this patch and then do the test again. Here's the result.
- 5.10 kernel with this patch not applied: avg = ~40s
- 5.10 kernel with this patch applied: avg = ~23s
Amazing! Note that all kernel is built with CONFIG_ARM64_PSEUDO_NMI not
set. It seems the pseudo-NMI feature actually brings some overhead to
performance event if CONFIG_ARM64_PSEUDO_NMI is not set.
Furthermore, I find the feature also brings some overhead to vmlinux size.
I build 5.10 kernel with this patch applied or not while
CONFIG_ARM64_PSEUDO_NMI is not set.
- 5.10 kernel with this patch not applied: vmlinux size is 384060600 Bytes.
- 5.10 kernel with this patch applied: vmlinux size is 383842936 Bytes.
That means arm64 pseudo-NMI feature may bring ~200KB overhead to
vmlinux size.
Above all, arm64 pseudo-NMI feature brings some overhead to vmlinux size
and performance even if config is not set. To avoid it, add macro control
all around the related code.
Signed-off-by: He Ying <heying24@huawei.com>
---
arch/arm64/include/asm/irqflags.h | 38 +++++++++++++++++++++++++++++--
arch/arm64/kernel/entry.S | 4 ++++
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
index b57b9b1e4344..82f771b41cf5 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -26,6 +26,7 @@
*/
static inline void arch_local_irq_enable(void)
{
+#ifdef CONFIG_ARM64_PSEUDO_NMI
if (system_has_prio_mask_debugging()) {
u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
@@ -41,10 +42,18 @@ static inline void arch_local_irq_enable(void)
: "memory");
pmr_sync();
+#else
+ asm volatile(
+ "msr daifclr, #3 // arch_local_irq_enable"
+ :
+ :
+ : "memory");
+#endif
}
static inline void arch_local_irq_disable(void)
{
+#ifdef CONFIG_ARM64_PSEUDO_NMI
if (system_has_prio_mask_debugging()) {
u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
@@ -58,6 +67,13 @@ static inline void arch_local_irq_disable(void)
:
: "r" ((unsigned long) GIC_PRIO_IRQOFF)
: "memory");
+#else
+ asm volatile(
+ "msr daifset, #3 // arch_local_irq_disable"
+ :
+ :
+ : "memory");
+#endif
}
/*
@@ -66,7 +82,7 @@ static inline void arch_local_irq_disable(void)
static inline unsigned long arch_local_save_flags(void)
{
unsigned long flags;
-
+#ifdef CONFIG_ARM64_PSEUDO_NMI
asm volatile(ALTERNATIVE(
"mrs %0, daif",
__mrs_s("%0", SYS_ICC_PMR_EL1),
@@ -74,12 +90,19 @@ static inline unsigned long arch_local_save_flags(void)
: "=&r" (flags)
:
: "memory");
-
+#else
+ asm volatile(
+ "mrs %0, daif"
+ : "=r" (flags)
+ :
+ : "memory");
+#endif
return flags;
}
static inline int arch_irqs_disabled_flags(unsigned long flags)
{
+#ifdef CONFIG_ARM64_PSEUDO_NMI
int res;
asm volatile(ALTERNATIVE(
@@ -91,6 +114,9 @@ static inline int arch_irqs_disabled_flags(unsigned long flags)
: "memory");
return res;
+#else
+ return flags & PSR_I_BIT;
+#endif
}
static inline int arch_irqs_disabled(void)
@@ -119,6 +145,7 @@ static inline unsigned long arch_local_irq_save(void)
*/
static inline void arch_local_irq_restore(unsigned long flags)
{
+#ifdef CONFIG_ARM64_PSEUDO_NMI
asm volatile(ALTERNATIVE(
"msr daif, %0",
__msr_s(SYS_ICC_PMR_EL1, "%0"),
@@ -128,6 +155,13 @@ static inline void arch_local_irq_restore(unsigned long flags)
: "memory");
pmr_sync();
+#else
+ asm volatile(
+ "msr daif, %0"
+ :
+ : "r" (flags)
+ : "memory");
+#endif
}
#endif /* __ASM_IRQFLAGS_H */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 2f69ae43941d..ffc32d3d909a 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -300,6 +300,7 @@ alternative_else_nop_endif
str w21, [sp, #S_SYSCALLNO]
.endif
+#ifdef CONFIG_ARM64_PSEUDO_NMI
/* Save pmr */
alternative_if ARM64_HAS_IRQ_PRIO_MASKING
mrs_s x20, SYS_ICC_PMR_EL1
@@ -307,6 +308,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING
mov x20, #GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET
msr_s SYS_ICC_PMR_EL1, x20
alternative_else_nop_endif
+#endif
/* Re-enable tag checking (TCO set on exception entry) */
#ifdef CONFIG_ARM64_MTE
@@ -330,6 +332,7 @@ alternative_else_nop_endif
disable_daif
.endif
+#ifdef CONFIG_ARM64_PSEUDO_NMI
/* Restore pmr */
alternative_if ARM64_HAS_IRQ_PRIO_MASKING
ldr x20, [sp, #S_PMR_SAVE]
@@ -339,6 +342,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING
dsb sy // Ensure priority change is seen by redistributor
.L__skip_pmr_sync\@:
alternative_else_nop_endif
+#endif
ldp x21, x22, [sp, #S_PC] // load ELR, SPSR
--
2.17.1
On Fri, 07 Jan 2022 08:55:36 +0000, He Ying <heying24@huawei.com> wrote: > > Our product has been updating its kernel from 4.4 to 5.10 recently and > found a performance issue. We do a bussiness test called ARP test, which > tests the latency for a ping-pong packets traffic with a certain payload. > The result is as following. > > - 4.4 kernel: avg = ~20s > - 5.10 kernel (CONFIG_ARM64_PSEUDO_NMI is not set): avg = ~40s > > I have been just learning arm64 pseudo-NMI code and have a question, > why is the related code not wrapped by CONFIG_ARM64_PSEUDO_NMI? > I wonder if this brings some performance regression. > > First, I make this patch and then do the test again. Here's the result. > > - 5.10 kernel with this patch not applied: avg = ~40s > - 5.10 kernel with this patch applied: avg = ~23s > > Amazing! Note that all kernel is built with CONFIG_ARM64_PSEUDO_NMI not > set. It seems the pseudo-NMI feature actually brings some overhead to > performance event if CONFIG_ARM64_PSEUDO_NMI is not set. > > Furthermore, I find the feature also brings some overhead to vmlinux size. > I build 5.10 kernel with this patch applied or not while > CONFIG_ARM64_PSEUDO_NMI is not set. > > - 5.10 kernel with this patch not applied: vmlinux size is 384060600 Bytes. > - 5.10 kernel with this patch applied: vmlinux size is 383842936 Bytes. > > That means arm64 pseudo-NMI feature may bring ~200KB overhead to > vmlinux size. > > Above all, arm64 pseudo-NMI feature brings some overhead to vmlinux size > and performance even if config is not set. To avoid it, add macro control > all around the related code. This obviously attracted my attention, and I took this patch for a ride on 5.16-rc8 on a machine that doesn't support GICv3 NMIs to make sure that any extra code would only result in pure overhead. There was no measurable difference with this patch applied or not, with CONFIG_ARM64_PSEUDO_NMI selected or not for the workloads I tried (I/O heavy virtual machines, hackbench). Mark already asked a number of questions (test case, implementation, test on a modern kernel). Please provide as many detail as you possibly can, because such a regression really isn't expected, and doesn't show up on the systems I have at hand. Some profiling numbers could also be interesting, in case this is a result of a particular resource being thrashed (TLB, cache...). Thanks, M. -- Without deviation from the norm, progress is not possible.
Hi Marc, I'm just back from the weekend and sorry for the delayed reply. 在 2022/1/8 20:51, Marc Zyngier 写道: > On Fri, 07 Jan 2022 08:55:36 +0000, > He Ying <heying24@huawei.com> wrote: >> Our product has been updating its kernel from 4.4 to 5.10 recently and >> found a performance issue. We do a bussiness test called ARP test, which >> tests the latency for a ping-pong packets traffic with a certain payload. >> The result is as following. >> >> - 4.4 kernel: avg = ~20s >> - 5.10 kernel (CONFIG_ARM64_PSEUDO_NMI is not set): avg = ~40s >> >> I have been just learning arm64 pseudo-NMI code and have a question, >> why is the related code not wrapped by CONFIG_ARM64_PSEUDO_NMI? >> I wonder if this brings some performance regression. >> >> First, I make this patch and then do the test again. Here's the result. >> >> - 5.10 kernel with this patch not applied: avg = ~40s >> - 5.10 kernel with this patch applied: avg = ~23s >> >> Amazing! Note that all kernel is built with CONFIG_ARM64_PSEUDO_NMI not >> set. It seems the pseudo-NMI feature actually brings some overhead to >> performance event if CONFIG_ARM64_PSEUDO_NMI is not set. >> >> Furthermore, I find the feature also brings some overhead to vmlinux size. >> I build 5.10 kernel with this patch applied or not while >> CONFIG_ARM64_PSEUDO_NMI is not set. >> >> - 5.10 kernel with this patch not applied: vmlinux size is 384060600 Bytes. >> - 5.10 kernel with this patch applied: vmlinux size is 383842936 Bytes. >> >> That means arm64 pseudo-NMI feature may bring ~200KB overhead to >> vmlinux size. >> >> Above all, arm64 pseudo-NMI feature brings some overhead to vmlinux size >> and performance even if config is not set. To avoid it, add macro control >> all around the related code. > This obviously attracted my attention, and I took this patch for a > ride on 5.16-rc8 on a machine that doesn't support GICv3 NMIs to make > sure that any extra code would only result in pure overhead. > > There was no measurable difference with this patch applied or not, > with CONFIG_ARM64_PSEUDO_NMI selected or not for the workloads I tried > (I/O heavy virtual machines, hackbench). Our test is some kind of network test. > > Mark already asked a number of questions (test case, implementation, > test on a modern kernel). Please provide as many detail as you > possibly can, because such a regression really isn't expected, and > doesn't show up on the systems I have at hand. Some profiling numbers > could also be interesting, in case this is a result of a particular > resource being thrashed (TLB, cache...). I replied to Mark a few moments ago and provided as many details as I can. You mentioned TLB and cache could be thrashed. How can we check this? By using perf tools? > > Thanks, > > M. >
Arm64 pseudo-NMI feature code brings some additional nops
when CONFIG_ARM64_PSEUDO_NMI is not set, which is not
necessary. So add necessary ifdeffery to avoid it.
Signed-off-by: He Ying <heying24@huawei.com>
---
arch/arm64/kernel/entry.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 2f69ae43941d..ffc32d3d909a 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -300,6 +300,7 @@ alternative_else_nop_endif
str w21, [sp, #S_SYSCALLNO]
.endif
+#ifdef CONFIG_ARM64_PSEUDO_NMI
/* Save pmr */
alternative_if ARM64_HAS_IRQ_PRIO_MASKING
mrs_s x20, SYS_ICC_PMR_EL1
@@ -307,6 +308,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING
mov x20, #GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET
msr_s SYS_ICC_PMR_EL1, x20
alternative_else_nop_endif
+#endif
/* Re-enable tag checking (TCO set on exception entry) */
#ifdef CONFIG_ARM64_MTE
@@ -330,6 +332,7 @@ alternative_else_nop_endif
disable_daif
.endif
+#ifdef CONFIG_ARM64_PSEUDO_NMI
/* Restore pmr */
alternative_if ARM64_HAS_IRQ_PRIO_MASKING
ldr x20, [sp, #S_PMR_SAVE]
@@ -339,6 +342,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING
dsb sy // Ensure priority change is seen by redistributor
.L__skip_pmr_sync\@:
alternative_else_nop_endif
+#endif
ldp x21, x22, [sp, #S_PC] // load ELR, SPSR
--
2.17.1
Hi all, Ping. Any comments? 在 2022/1/12 11:24, He Ying 写道: > Arm64 pseudo-NMI feature code brings some additional nops > when CONFIG_ARM64_PSEUDO_NMI is not set, which is not > necessary. So add necessary ifdeffery to avoid it. > > Signed-off-by: He Ying <heying24@huawei.com> > --- > arch/arm64/kernel/entry.S | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > index 2f69ae43941d..ffc32d3d909a 100644 > --- a/arch/arm64/kernel/entry.S > +++ b/arch/arm64/kernel/entry.S > @@ -300,6 +300,7 @@ alternative_else_nop_endif > str w21, [sp, #S_SYSCALLNO] > .endif > > +#ifdef CONFIG_ARM64_PSEUDO_NMI > /* Save pmr */ > alternative_if ARM64_HAS_IRQ_PRIO_MASKING > mrs_s x20, SYS_ICC_PMR_EL1 > @@ -307,6 +308,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING > mov x20, #GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET > msr_s SYS_ICC_PMR_EL1, x20 > alternative_else_nop_endif > +#endif > > /* Re-enable tag checking (TCO set on exception entry) */ > #ifdef CONFIG_ARM64_MTE > @@ -330,6 +332,7 @@ alternative_else_nop_endif > disable_daif > .endif > > +#ifdef CONFIG_ARM64_PSEUDO_NMI > /* Restore pmr */ > alternative_if ARM64_HAS_IRQ_PRIO_MASKING > ldr x20, [sp, #S_PMR_SAVE] > @@ -339,6 +342,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING > dsb sy // Ensure priority change is seen by redistributor > .L__skip_pmr_sync\@: > alternative_else_nop_endif > +#endif > > ldp x21, x22, [sp, #S_PC] // load ELR, SPSR >
On Wed, Jan 19, 2022 at 02:40:58PM +0800, He Ying wrote: > Hi all, > > Ping. Any comments? The patch looks fine, but as it's the middle of the merge window people are busy and unlikely to look at this for the next few days. Generally it's a good idea to wait until rc1 or rc2, rebase atop that, and post the updated patch. Stuff like this usually gets queued around rc3/rc4 time. > 锟斤拷 2022/1/12 11:24, He Ying 写锟斤拷: > > Arm64 pseudo-NMI feature code brings some additional nops > > when CONFIG_ARM64_PSEUDO_NMI is not set, which is not > > necessary. So add necessary ifdeffery to avoid it. > > > > Signed-off-by: He Ying <heying24@huawei.com> FWIW: Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > > --- > > arch/arm64/kernel/entry.S | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > > index 2f69ae43941d..ffc32d3d909a 100644 > > --- a/arch/arm64/kernel/entry.S > > +++ b/arch/arm64/kernel/entry.S > > @@ -300,6 +300,7 @@ alternative_else_nop_endif > > str w21, [sp, #S_SYSCALLNO] > > .endif > > +#ifdef CONFIG_ARM64_PSEUDO_NMI > > /* Save pmr */ > > alternative_if ARM64_HAS_IRQ_PRIO_MASKING > > mrs_s x20, SYS_ICC_PMR_EL1 > > @@ -307,6 +308,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING > > mov x20, #GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET > > msr_s SYS_ICC_PMR_EL1, x20 > > alternative_else_nop_endif > > +#endif > > /* Re-enable tag checking (TCO set on exception entry) */ > > #ifdef CONFIG_ARM64_MTE > > @@ -330,6 +332,7 @@ alternative_else_nop_endif > > disable_daif > > .endif > > +#ifdef CONFIG_ARM64_PSEUDO_NMI > > /* Restore pmr */ > > alternative_if ARM64_HAS_IRQ_PRIO_MASKING > > ldr x20, [sp, #S_PMR_SAVE] > > @@ -339,6 +342,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING > > dsb sy // Ensure priority change is seen by redistributor > > .L__skip_pmr_sync\@: > > alternative_else_nop_endif > > +#endif > > ldp x21, x22, [sp, #S_PC] // load ELR, SPSR
在 2022/1/19 17:35, Mark Rutland 写道: > On Wed, Jan 19, 2022 at 02:40:58PM +0800, He Ying wrote: >> Hi all, >> >> Ping. Any comments? > The patch looks fine, but as it's the middle of the merge window people > are busy and unlikely to look at this for the next few days. > > Generally it's a good idea to wait until rc1 or rc2, rebase atop that, > and post the updated patch. Stuff like this usually gets queued around > rc3/rc4 time. OK. Thanks for your comment. > >> 锟斤拷 2022/1/12 11:24, He Ying 写锟斤拷: >>> Arm64 pseudo-NMI feature code brings some additional nops >>> when CONFIG_ARM64_PSEUDO_NMI is not set, which is not >>> necessary. So add necessary ifdeffery to avoid it. >>> >>> Signed-off-by: He Ying <heying24@huawei.com> > FWIW: > > Acked-by: Mark Rutland <mark.rutland@arm.com> > > Mark. > >>> --- >>> arch/arm64/kernel/entry.S | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S >>> index 2f69ae43941d..ffc32d3d909a 100644 >>> --- a/arch/arm64/kernel/entry.S >>> +++ b/arch/arm64/kernel/entry.S >>> @@ -300,6 +300,7 @@ alternative_else_nop_endif >>> str w21, [sp, #S_SYSCALLNO] >>> .endif >>> +#ifdef CONFIG_ARM64_PSEUDO_NMI >>> /* Save pmr */ >>> alternative_if ARM64_HAS_IRQ_PRIO_MASKING >>> mrs_s x20, SYS_ICC_PMR_EL1 >>> @@ -307,6 +308,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING >>> mov x20, #GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET >>> msr_s SYS_ICC_PMR_EL1, x20 >>> alternative_else_nop_endif >>> +#endif >>> /* Re-enable tag checking (TCO set on exception entry) */ >>> #ifdef CONFIG_ARM64_MTE >>> @@ -330,6 +332,7 @@ alternative_else_nop_endif >>> disable_daif >>> .endif >>> +#ifdef CONFIG_ARM64_PSEUDO_NMI >>> /* Restore pmr */ >>> alternative_if ARM64_HAS_IRQ_PRIO_MASKING >>> ldr x20, [sp, #S_PMR_SAVE] >>> @@ -339,6 +342,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING >>> dsb sy // Ensure priority change is seen by redistributor >>> .L__skip_pmr_sync\@: >>> alternative_else_nop_endif >>> +#endif >>> ldp x21, x22, [sp, #S_PC] // load ELR, SPSR > .
On Tue, 11 Jan 2022 22:24:10 -0500, He Ying wrote:
> Arm64 pseudo-NMI feature code brings some additional nops
> when CONFIG_ARM64_PSEUDO_NMI is not set, which is not
> necessary. So add necessary ifdeffery to avoid it.
>
>
Applied to arm64 (for-next/misc), thanks!
[1/1] arm64: entry: Save some nops when CONFIG_ARM64_PSEUDO_NMI is not set
https://git.kernel.org/arm64/c/3352a5556f52
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
© 2016 - 2026 Red Hat, Inc.