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

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

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 <sshegde@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);
>