All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.