public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Ilvokhin <d@ilvokhin.com>
To: Thomas Gleixner <tglx@kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, Neil Horman <nhorman@tuxdriver.com>
Subject: Re: [patch 04/14] x86/irq: Make irqstats array based
Date: Mon, 9 Mar 2026 18:12:30 +0000	[thread overview]
Message-ID: <aa8NjoGSgF8Eq_iy@shell.ilvokhin.com> (raw)
In-Reply-To: <20260303154548.218256740@kernel.org>

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@kernel.org>
> ---
>  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.

  parent reply	other threads:[~2026-03-09 18:12 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-04 18:55 [patch 00/14] genirq: Improve /proc/interrupts for real and add a binary interface Thomas Gleixner
2026-03-04 18:55 ` [patch 01/14] x86/irq: Optimize interrupts decimals printing Thomas Gleixner
2026-03-04 18:55 ` [patch 02/14] genirq/proc: Avoid formatting zero counts in /proc/interrupts Thomas Gleixner
2026-03-09 15:59   ` Dmitry Ilvokhin
2026-03-04 18:55 ` [patch 03/14] genirq/proc: Utilize irq_desc::tot_count to avoid evaluation Thomas Gleixner
2026-03-09 16:04   ` Dmitry Ilvokhin
2026-03-04 18:55 ` [patch 04/14] x86/irq: Make irqstats array based Thomas Gleixner
2026-03-04 22:18   ` Michael Kelley
2026-03-05 15:52     ` Thomas Gleixner
2026-03-09 18:12   ` Dmitry Ilvokhin [this message]
2026-03-10 10:15     ` Thomas Gleixner
2026-03-04 18:55 ` [patch 05/14] genirq: Expose nr_irqs in core code Thomas Gleixner
2026-03-09 18:26   ` Dmitry Ilvokhin
2026-03-04 18:55 ` [patch 06/14] genirq: Cache the condition for /proc/interrupts exposure Thomas Gleixner
2026-03-16 18:46   ` Dmitry Ilvokhin
2026-03-04 18:55 ` [patch 07/14] genirq: Calculate precision only when required Thomas Gleixner
2026-03-16 18:57   ` Dmitry Ilvokhin
2026-03-04 18:56 ` [patch 08/14] genirq: Add rcuref count to struct irq_desc Thomas Gleixner
2026-03-04 18:56 ` [patch 09/14] genirq: Expose irq_find_desc_at_or_after() in core code Thomas Gleixner
2026-03-04 18:56 ` [patch 10/14] genirq/proc: Speed up /proc/interrupts iteration Thomas Gleixner
2026-03-04 18:56 ` [patch 11/14] [RFC] genirq: Cache target CPU for single CPU affinities Thomas Gleixner
2026-03-04 18:56 ` [patch 12/14] [RFC] genirq/proc: Provide binary statistic interface Thomas Gleixner
2026-03-04 18:56 ` [patch 13/14] [RFC] genirq/proc: Provide architecture specific binary statistics Thomas Gleixner
2026-03-04 18:56 ` [patch 14/14] [RFC] x86/irq: Hook up architecture specific stats Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aa8NjoGSgF8Eq_iy@shell.ilvokhin.com \
    --to=d@ilvokhin.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=tglx@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox