2024-06-07 07:45:45

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH] arm64: smp: Fix missing IPI statistics

commit 83cfac95c018 ("genirq: Allow interrupts to be excluded from
/proc/interrupts") is to avoid IPIs appear twice in /proc/interrupts.
But the commit 331a1b3a836c ("arm64: smp: Add arch support for backtrace
using pseudo-NMI") and commit 2f5cd0c7ffde("arm64: kgdb: Implement
kgdb_roundup_cpus() to enable pseudo-NMI roundup") set CPU_BACKTRACE and
KGDB_ROUNDUP IPIs "IRQ_HIDDEN" flag but not show them in
arch_show_interrupts(), which cause the interrupt kstat_irqs accounting
is missing in display.

Fixes: 331a1b3a836c ("arm64: smp: Add arch support for backtrace using pseudo-NMI")
Signed-off-by: Jinjie Ruan <[email protected]>
---
arch/arm64/kernel/smp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 31c8b3094dd7..7f9a5cf0f3b8 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -1039,7 +1039,8 @@ void __init set_smp_ipi_range(int ipi_base, int n)
}

ipi_desc[i] = irq_to_desc(ipi_base + i);
- irq_set_status_flags(ipi_base + i, IRQ_HIDDEN);
+ if (i < NR_IPI)
+ irq_set_status_flags(ipi_base + i, IRQ_HIDDEN);
}

ipi_irq_base = ipi_base;
--
2.34.1



2024-06-07 16:04:11

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] arm64: smp: Fix missing IPI statistics

Hi,

On Fri, Jun 7, 2024 at 12:45 AM Jinjie Ruan <[email protected]> wrote:
>
> commit 83cfac95c018 ("genirq: Allow interrupts to be excluded from
> /proc/interrupts") is to avoid IPIs appear twice in /proc/interrupts.
> But the commit 331a1b3a836c ("arm64: smp: Add arch support for backtrace
> using pseudo-NMI") and commit 2f5cd0c7ffde("arm64: kgdb: Implement
> kgdb_roundup_cpus() to enable pseudo-NMI roundup") set CPU_BACKTRACE and
> KGDB_ROUNDUP IPIs "IRQ_HIDDEN" flag but not show them in
> arch_show_interrupts(), which cause the interrupt kstat_irqs accounting
> is missing in display.
>
> Fixes: 331a1b3a836c ("arm64: smp: Add arch support for backtrace using pseudo-NMI")
> Signed-off-by: Jinjie Ruan <[email protected]>
> ---
> arch/arm64/kernel/smp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)

While I won't object to your patch if everyone agrees that we want it,
fully excluding "cpu backtrace" and "kgdb roundup" from
/proc/interrupts was more of a design decision than a bug. Those two
IPIs are really special cases and not something that I'd expect anyone
to care about knowing the count of. Keeping them out of
"/proc/interrupts" just avoids noise. I'd also note that I believe
arm32 makes the same design choice for "cpu backtrace".

In any case, if we truly think people want the count of these IPIs
then it feels like we should report them in arch_show_interrupts()
where we can give them a nice string.

-Doug

2024-06-14 02:51:42

by Jinjie Ruan

[permalink] [raw]
Subject: Re: [PATCH] arm64: smp: Fix missing IPI statistics



On 2024/6/8 0:02, Doug Anderson wrote:
> Hi,
>
> On Fri, Jun 7, 2024 at 12:45 AM Jinjie Ruan <[email protected]> wrote:
>>
>> commit 83cfac95c018 ("genirq: Allow interrupts to be excluded from
>> /proc/interrupts") is to avoid IPIs appear twice in /proc/interrupts.
>> But the commit 331a1b3a836c ("arm64: smp: Add arch support for backtrace
>> using pseudo-NMI") and commit 2f5cd0c7ffde("arm64: kgdb: Implement
>> kgdb_roundup_cpus() to enable pseudo-NMI roundup") set CPU_BACKTRACE and
>> KGDB_ROUNDUP IPIs "IRQ_HIDDEN" flag but not show them in
>> arch_show_interrupts(), which cause the interrupt kstat_irqs accounting
>> is missing in display.
>>
>> Fixes: 331a1b3a836c ("arm64: smp: Add arch support for backtrace using pseudo-NMI")
>> Signed-off-by: Jinjie Ruan <[email protected]>
>> ---
>> arch/arm64/kernel/smp.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> While I won't object to your patch if everyone agrees that we want it,

Hello, What's everyone's opinion?

> fully excluding "cpu backtrace" and "kgdb roundup" from
> /proc/interrupts was more of a design decision than a bug. Those two
> IPIs are really special cases and not something that I'd expect anyone
> to care about knowing the count of. Keeping them out of
> "/proc/interrupts" just avoids noise. I'd also note that I believe
> arm32 makes the same design choice for "cpu backtrace".

Yes, arm32 is same as arm64.

>
> In any case, if we truly think people want the count of these IPIs
> then it feels like we should report them in arch_show_interrupts()
> where we can give them a nice string.

That's a good idea.

>
> -Doug