Hi David,
On Wed, Nov 20, 2024 at 3:08 AM David Wang <00107082@xxxxxxx> wrote:
> At 2024-11-20 09:37:04, "David Wang" <00107082@xxxxxxx> wrote:
> >At 2024-11-20 03:55:30, "Geert Uytterhoeven" <geert@xxxxxxxxxxxxxx> wrote:
> >>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@xxxxxxx>
> >>
> >>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...)
>
> Several drivers use dev_name as format string for seq_printf, would this raise security concerns?
>
> drivers/gpio/gpio-xgs-iproc.c: seq_printf(p, dev_name(chip->dev));
> drivers/gpio/gpio-mlxbf2.c: seq_printf(p, dev_name(gs->dev));
> drivers/gpio/gpio-omap.c: seq_printf(p, dev_name(bank->dev));
> drivers/gpio/gpio-hlwd.c: seq_printf(p, dev_name(hlwd->dev));
> drivers/gpio/gpio-aspeed.c: seq_printf(p, dev_name(gpio->dev));
> drivers/gpio/gpio-pca953x.c: seq_printf(p, dev_name(gc->parent));
> drivers/gpio/gpio-tegra186.c: seq_printf(p, dev_name(gc->parent));
> drivers/gpio/gpio-tegra.c: seq_printf(s, dev_name(chip->parent));
> drivers/gpio/gpio-ep93xx.c: seq_printf(p, dev_name(gc->parent));
> drivers/gpio/gpio-aspeed-sgpio.c: seq_printf(p, dev_name(gpio->dev));
> drivers/gpio/gpio-pl061.c: seq_printf(p, dev_name(gc->parent));
> drivers/gpio/gpio-visconti.c: seq_printf(p, dev_name(priv->dev));
In theory, yes. But I guess it's hard to sneak a percent sign in these
device names.
But given the above, all of them should probably be updated to print
an initial space?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
