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