Re: [patch 04/14] x86/irq: Make irqstats array based

[Dmitry Ilvokhin]

On Wed, Mar 04, 2026 at 07:55:45PM +0100, Thomas Gleixner wrote:
> Having the x86 specific interrupt statistics as a data structure with
> individual members instead of an array is just stupid as it requires
> endless copy and paste in arch_show_interrupts() and arch_irq_stat_cpu(),
> where the latter does not even take the latest interrupt additions into
> account. The resulting #ifdef orgy is just disgusting.
> 
> Convert it to an array of counters, which does not make a difference in the
> actual interrupt hotpath increment as the array index is constant and
> therefore not any different than the member based access.
> 
> But in arch_show_interrupts() and arch_irq_stat_cpu() this just turns into
> a loop, which reduces the text size by ~2k (~12%):
> 
>    text	   data	    bss	    dec	    hex	filename
>   19643	  15250	    904	  35797	   8bd5	../build/arch/x86/kernel/irq.o
>   17355	  15250	    904	  33509	   82e5	../build/arch/x86/kernel/irq.o
> 
> Adding a new vector or software counter only requires to update the table
> and everything just works. Using the core provided emit function which
> speeds up 0 outputs makes it significantly faster.
> 
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxx>
> ---
>  arch/x86/events/amd/core.c          |    2 
>  arch/x86/events/amd/ibs.c           |    2 
>  arch/x86/events/core.c              |    2 
>  arch/x86/events/intel/core.c        |    2 
>  arch/x86/events/intel/knc.c         |    2 
>  arch/x86/events/intel/p4.c          |    2 
>  arch/x86/events/zhaoxin/core.c      |    2 
>  arch/x86/hyperv/hv_init.c           |    2 
>  arch/x86/include/asm/hardirq.h      |   69 ++++++----
>  arch/x86/include/asm/mce.h          |    3 
>  arch/x86/kernel/apic/apic.c         |    4 
>  arch/x86/kernel/apic/ipi.c          |    2 
>  arch/x86/kernel/cpu/acrn.c          |    2 
>  arch/x86/kernel/cpu/mce/amd.c       |    2 
>  arch/x86/kernel/cpu/mce/core.c      |    8 -
>  arch/x86/kernel/cpu/mce/threshold.c |    2 
>  arch/x86/kernel/cpu/mshyperv.c      |    4 
>  arch/x86/kernel/irq.c               |  227 ++++++++++--------------------------
>  arch/x86/kernel/irq_work.c          |    2 
>  arch/x86/kernel/kvm.c               |    2 
>  arch/x86/kernel/nmi.c               |    4 
>  arch/x86/kernel/smp.c               |    6 
>  arch/x86/mm/tlb.c                   |    2 
>  arch/x86/xen/enlighten_hvm.c        |    2 
>  arch/x86/xen/enlighten_pv.c         |    2 
>  arch/x86/xen/smp.c                  |    6 
>  arch/x86/xen/smp_pv.c               |    2 
>  27 files changed, 135 insertions(+), 232 deletions(-)
> 

[...]

> -	if (x86_platform_ipi_callback) {
> -		seq_printf(p, "%*s:", prec, "PLT");
> -		for_each_online_cpu(j)
> -			put_decimal(p, irq_stats(j)->x86_platform_ipis);
> -		seq_puts(p, "  Platform interrupts\n");
> -	}
> +	ISS(APIC_TIMER,			"LOC", " Local timer interrupts\n"),
> +	ISS(SPURIOUS,			"SPU", " Spurious interrupts\n"),
> +	ISS(APIC_PERF,			"PMI", " Performance monitoring interrupts\n"),
> +	ISS(IRQ_WORK,			"IWI", " IRQ work interrupts\n"),
> +	ISS(ICR_READ_RETRY,		"RTR", " APIC ICR read retries\n"),
> +	ISS(X86_PLATFORM_IPI,		"PLT", " Platform interrupts\n"),

The old code only showed "PLT" when x86_platform_ipi_callback was set,
but with ISS() this is now unconditional.

Is this intentional?

[...]

> -	seq_printf(p, "%*s: %10u\n", prec, "ERR", atomic_read(&irq_err_count));
> -#if defined(CONFIG_X86_IO_APIC)
> -	seq_printf(p, "%*s: %10u\n", prec, "MIS", atomic_read(&irq_mis_count));
> +	ITS(HYPERV_REENLIGHTMENT,	"HRE", " Hyper-V reenlightment interrupts\n"),

HYPERV_REENLIGHTMENT doesn't match the enum (HYPERV_REENLIGHTENMENT).
This should break the build with CONFIG_HYPERV=y. There is also a same
typo in text description.

> +	seq_printf(p, "ERR: %10u\n", (unsigned int) atomic_read(&irq_err_count));
> +	if (IS_ENABLED(CONFIG_X86_IO_APIC))
> +		seq_printf(p, "MIS: %10u\n", (unsigned int) atomic_read(&irq_mis_count));

This drops the prec-based alignment for ERR and MIS.