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

["David Wang" <00107082@163.com>]

Hi, 
At 2024-11-20 03:55:30, "Geert Uytterhoeven" <geert@linux-m68k.org> wrote:
> 	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.

Thanks for reporting this, I was considering the spaces and checked it on my system,
I thought "all" descriptions have leading spaces and it's ok to remove the extra one.
But I did not check all the "irq_print_chip" codes, now when
checking the code, there are many GPIO drivers' implementations with no leading spaces.
(The behavior is not consistent cross  driver implementations though...)

Sorry for the regression, and thanks for catching this.


David