| Message ID | 20240607074716.4068975-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: smp: Fix missing IPI statistics | expand |
Hi, On Fri, Jun 7, 2024 at 12:45 AM Jinjie Ruan <ruanjinjie@huawei.com> 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 <ruanjinjie@huawei.com> > --- > 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
On 2024/6/8 0:02, Doug Anderson wrote: > Hi, > > On Fri, Jun 7, 2024 at 12:45 AM Jinjie Ruan <ruanjinjie@huawei.com> 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 <ruanjinjie@huawei.com> >> --- >> 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
On Fri, Jun 07, 2024 at 03:47:16PM +0800, Jinjie Ruan 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 <ruanjinjie@huawei.com> > --- > 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); > } Please can you show the contents of /proc/interrupts before and after this patch and put that in the commit message? I'm not seeing how the two new IPIs get picked up by arch_show_interrupts(). Thanks, Will
On 2024/6/19 21:32, Will Deacon wrote: > On Fri, Jun 07, 2024 at 03:47:16PM +0800, Jinjie Ruan 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 <ruanjinjie@huawei.com> >> --- >> 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); >> } > > Please can you show the contents of /proc/interrupts before and after > this patch and put that in the commit message? I'm not seeing how the > two new IPIs get picked up by arch_show_interrupts(). Before this patch, CPU_BACKTRACE and KGDB_ROUNDUP IPIs are missing as below: / # cat /proc/interrupts CPU0 CPU1 CPU2 CPU3 11: 463 656 243 654 GICv3 27 Level arch_timer 13: 14 0 0 0 GICv3 33 Level uart-pl011 17: 0 0 0 0 ITS-MSI 16384 Edge virtio2-config 18: 0 5 0 0 ITS-MSI 16385 Edge virtio2-input 19: 68 0 0 0 GICv3 78 Edge virtio0 20: 0 0 0 0 GICv3 79 Edge virtio1 21: 0 0 0 0 GICv3 34 Level rtc-pl031 22: 2 2 2 2 GICv3 23 Level arm-pmu 23: 0 0 0 0 ITS-MSI 32768 Edge virtio3-config 24: 0 0 0 0 ITS-MSI 32769 Edge virtio3-requests 25: 0 0 0 0 9030000.pl061 3 Edge GPIO Key Poweroff IPI0: 15 13 5 21 Rescheduling interrupts IPI1: 385 129 282 194 Function call interrupts IPI2: 0 0 0 0 CPU stop interrupts IPI3: 0 0 0 0 CPU stop (for crash dump) interrupts IPI4: 0 0 0 0 Timer broadcast interrupts IPI5: 1 0 0 0 IRQ work interrupts Err: 0 After this patch the hwirq6/7 IPI is also displayed: / # cat /proc/interrupts CPU0 CPU1 CPU2 CPU3 7: 0 0 0 0 GICv3 6 Edge IPI 8: 0 0 0 0 GICv3 7 Edge IPI 11: 414 433 424 422 GICv3 27 Level arch_timer 13: 21 0 0 0 GICv3 33 Level uart-pl011 17: 0 0 0 0 ITS-MSI 16384 Edge virtio2-config 18: 0 6 0 0 ITS-MSI 16385 Edge virtio2-input 19: 60 0 0 0 GICv3 78 Edge virtio0 20: 0 0 0 0 GICv3 79 Edge virtio1 21: 0 0 0 0 GICv3 34 Level rtc-pl031 22: 3 3 3 3 GICv3 23 Level arm-pmu 23: 0 0 0 0 ITS-MSI 32768 Edge virtio3-config 24: 0 0 0 0 ITS-MSI 32769 Edge virtio3-requests 25: 0 0 0 0 9030000.pl061 3 Edge GPIO Key Poweroff IPI0: 15 13 19 12 Rescheduling interrupts IPI1: 241 162 328 252 Function call interrupts IPI2: 0 0 0 0 CPU stop interrupts IPI3: 0 0 0 0 CPU stop (for crash dump) interrupts IPI4: 0 0 0 0 Timer broadcast interrupts IPI5: 0 1 0 0 IRQ work interrupts Err: 0 > > Thanks, > > Will
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;
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 <ruanjinjie@huawei.com> --- arch/arm64/kernel/smp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)