Re: [PATCH 3/3] powerpc/irq: Suppress unlikely interrupt stats by default

[Christophe Leroy (CS GROUP)]

Le 23/05/2026 à 19:40, Shrikanth Hegde a écrit :
Some interrupts are always zero and that is expected since they occur
very rarely and are mostly error indications. Don't print them by
default.

"MCE" -  "Machine check exceptions"
"NMI" -  "System Reset interrupts"

Print them if they occur once. Maintain a bitmap to know which
interrupts are to be printed.

Is that bitmap needed at all ? Can't we just print them as soon as they 
are not zero ?


Time taken to read /proc/interrupts 1000 times.
Base and v6 details can be found in cover-letter.
Base            :  103us
v6              :   63us
v6+patch 1+2    :   57us
v6+patch 1+2+3  :   54us

Patch 3 shows an additional 5% gain compared to patch 1+2. So it does
make sense to print them only if they are ever set.

Note: Since /proc/interrupts depend on kconfig and arch dependent,
userspace tools don't make explicit assumptions.

Signed-off-by: Shrikanth Hegde <sshe...@linux.ibm.com>
---
  arch/powerpc/include/asm/hardirq.h |  1 +
  arch/powerpc/kernel/irq.c          | 37 +++++++++++++++++++++++++++---
  arch/powerpc/kernel/traps.c        |  4 ++--
  3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/hardirq.h 
b/arch/powerpc/include/asm/hardirq.h
index 38098e35b241..be6cd5aab016 100644
--- a/arch/powerpc/include/asm/hardirq.h
+++ b/arch/powerpc/include/asm/hardirq.h
@@ -31,6 +31,7 @@ DECLARE_PER_CPU(unsigned int, __softirq_pending);
  #define local_softirq_pending_ref       __softirq_pending
  
  #define inc_irq_stat(index)	__this_cpu_inc(irq_stat.counts[IRQ_COUNT_##index])
+void inc_irq_stat_and_enable(enum irq_stat_counts which);
  
  #define __ARCH_IRQ_STAT
  #define __ARCH_IRQ_EXIT_IRQS_DISABLED
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index e67a18f62142..048ddfa66fc4 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -87,9 +87,13 @@ u32 tau_interrupts(unsigned long cpu);
  struct irq_stat_info {
        const char      *symbol;
        const char      *text;
+       int             skip;

I'd call it 'optional' instead, and then during the print, if value 0 
and optional then don't print.

  };
  
-#define ISE(idx, sym, txt)[IRQ_COUNT_##idx] = { .symbol = sym, .text = txt}
+/* ISE - IRQ STAT ENABLED, ISC - IRQ STAT CONDITIONAL */
+#define ISE(idx, sym, txt)[IRQ_COUNT_##idx] = { .symbol = sym, .text = txt, 
.skip = 0}
+#define ISC(idx, sym, txt)[IRQ_COUNT_##idx] = { .symbol = sym, .text = txt, 
.skip = 1}
+
  
  static struct irq_stat_info irq_stat_info[IRQ_COUNT_MAX] __ro_after_init = {
        ISE(LOC_TIMER,          "LOC", "  Local timer interrupts for timer event 
device\n"),
@@ -97,8 +101,8 @@ static struct irq_stat_info irq_stat_info[IRQ_COUNT_MAX] 
__ro_after_init = {
        ISE(OTHER_TIMER,        "LOC", "  Local timer interrupts for others\n"),
        ISE(SPURIOUS,           "SPU", "  Spurious interrupts\n"),
        ISE(PMI,                "PMI", "  Performance monitoring interrupts\n"),
-       ISE(MCE,                "MCE", "  Machine check exceptions\n"),
-       ISE(NMI_SRESET,         "NMI", "  System Reset interrupts\n"),
+       ISC(MCE,                "MCE", "  Machine check exceptions\n"),
+       ISC(NMI_SRESET,         "NMI", "  System Reset interrupts\n"),
  #ifdef CONFIG_PPC_WATCHDOG
        ISE(WATCHDOG,           "WDG", "  Watchdog soft-NMI interrupts\n"),
  #endif
@@ -107,11 +111,25 @@ static struct irq_stat_info irq_stat_info[IRQ_COUNT_MAX] 
__ro_after_init = {
  #endif
  };
  
+/*
+ * Used for default disabled counters to increment the stats and to enable the
+ * entry for /proc/interrupts output.
+ */
+static DECLARE_BITMAP(irq_stat_count_show, IRQ_COUNT_MAX) __read_mostly;
+void inc_irq_stat_and_enable(enum irq_stat_counts which)
+{
+       __this_cpu_inc(irq_stat.counts[which]);
+       set_bit(which, irq_stat_count_show);
+}
+
  int arch_show_interrupts(struct seq_file *p, int prec)
  {
        const struct irq_stat_info *info = irq_stat_info;
  
  	for (unsigned int i = 0; i < ARRAY_SIZE(irq_stat_info); i++, info++) {
+               if (!test_bit(i, irq_stat_count_show))
+                       continue;
+
                seq_printf(p, "%*s:", prec, info->symbol);
                irq_proc_emit_counts(p, &irq_stat.counts[i]);
                seq_puts(p, info->text);
@@ -138,6 +156,19 @@ int arch_show_interrupts(struct seq_file *p, int prec)
        return 0;
  }
  
+static int __init irq_init_stats(void)
+{
+       struct irq_stat_info *info = irq_stat_info;
+
+       for (unsigned int i = 0; i < ARRAY_SIZE(irq_stat_info); i++, info++) {
+               if (info->skip == 0)
+                       set_bit(i, irq_stat_count_show);
+       }
+
+       return 0;
+}
+late_initcall(irq_init_stats);
+
  /*
   * /proc/stat helpers
   */
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index a8f15154bd9a..3eacbd20fc80 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -459,7 +459,7 @@ DEFINE_INTERRUPT_HANDLER_NMI(system_reset_exception)
        }
  
  	hv_nmi_check_nonrecoverable(regs);
-       inc_irq_stat(NMI_SRESET);
+       inc_irq_stat_and_enable(IRQ_COUNT_NMI_SRESET);
  
  	/* See if any machine dependent calls */
        if (ppc_md.system_reset_exception) {
@@ -816,7 +816,7 @@ static void __machine_check_exception(struct pt_regs *regs)
  {
        int recover = 0;
  
-	inc_irq_stat(MCE);
+       inc_irq_stat_and_enable(IRQ_COUNT_MCE);
  
  	add_taint(TAINT_MACHINE_CHECK, LOCKDEP_NOW_UNRELIABLE);