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

Fri, 29 May 2026 01:09:45 -0700 

Hi Christophe.
Thanks for reviewing the patch series.

On 5/29/26 1:21 PM, Christophe Leroy (CS GROUP) wrote:



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 ?
I think Yes. Otherwise need to traverse all the per_cpu count and to see if it is zero or not. 




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 <[email protected]>
---
  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. 



Ok. Makes sense. Will do it in v2.

Will add below way

        if (!(info->optional && test_bit(i, irq_stat_count_show)))
            continue;


  };
-#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);

Reply via email to