Re: [PATCH 01/13] kernel/irq/proc: use seq_put_decimal_ull_width() for decimal values

[Geert Uytterhoeven <geert@linux-m68k.org>]

 	Hi David,

On Sat, 9 Nov 2024, David Wang wrote:
> seq_printf() is costy, on a system with m interrupts and n CPUs, there
> would be m*n decimal values yield via seq_printf() when reading
> /proc/interrupts, the cost parsing format strings grows with number of
> CPU. Profiling on a x86 8-core system indicates seq_printf() takes ~47%
> samples of show_interrupts(), and replace seq_printf() with
> seq_put_decimal_ull_width() could have near 30% performance gain.
>
> The improvement has pratical significance, considering many monitoring
> tools would read /proc/interrupts periodically.
>
> Signed-off-by: David Wang <00107082@163.com>

Thanks for your patch, which is now commit f9ed1f7c2e26fcd1
("genirq/proc: Use seq_put_decimal_ull_width() for decimal values")
in irqchip/irq/core.

This removes a space after the last CPU column, causing the values in
this column to be concatenated to the values in the next column.

E.g. on Koelsch (R-Car M-W), the output changes from:

 	       CPU0       CPU1
      27:       1871       2017 GIC-0  27 Level     arch_timer
      29:        646          0 GIC-0 205 Level     e60b0000.i2c
      30:          0          0 GIC-0 174 Level     ffca0000.timer
      31:          0          0 GIC-0  36 Level     e6050000.gpio
      32:          0          0 GIC-0  37 Level     e6051000.gpio
      [...]

to

 	       CPU0       CPU1
      27:       1966       1900GIC-0  27 Level     arch_timer
      29:        580          0GIC-0 205 Level     e60b0000.i2c
      30:          0          0GIC-0 174 Level     ffca0000.timer
      31:          0          0GIC-0  36 Level     e6050000.gpio
      32:          0          0GIC-0  37 Level     e6051000.gpio
      [...]

making the output hard to read, and probably breaking scripts that parse
its contents.

Reverting the commit fixes the issue for me.

> --- a/kernel/irq/proc.c
> +++ b/kernel/irq/proc.c
> @@ -494,9 +494,11 @@ int show_interrupts(struct seq_file *p, void *v)
> 	if (!desc->action || irq_desc_is_chained(desc) || !desc->kstat_irqs)
> 		goto outsparse;
>
> -	seq_printf(p, "%*d: ", prec, i);
> +	seq_printf(p, "%*d:", prec, i);
> 	for_each_online_cpu(j)
> -		seq_printf(p, "%10u ", desc->kstat_irqs ? per_cpu(desc->kstat_irqs->cnt, j) : 0);
> +		seq_put_decimal_ull_width(p, " ",
> +					  desc->kstat_irqs ? per_cpu(desc->kstat_irqs->cnt, j) : 0,
> +					  10);
>
> 	raw_spin_lock_irqsave(&desc->lock, flags);
> 	if (desc->irq_data.chip) {

Gr{oetje,eeting}s,

 						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
 							    -- Linus Torvalds