Re: [tip: timers/core] timer_list: Use printk format instead of open-coded symbol lookup

Joe Perches
2020-11-16T01:48:26Z

On Sun, 2020-11-15 at 22:51 +0000, tip-bot2 for Helge Deller wrote:
> The following commit has been merged into the timers/core branch of tip:
>
> Commit-ID: da88f9b3113620dcd30fc203236aa53d5430ee98
> Gitweb: https://git.kernel.org/tip/da88f9b3113620dcd30fc203236aa53d5430ee98
> Author: Helge Deller <deller@gmx.de>
> AuthorDate: Wed, 04 Nov 2020 17:34:01 +01:00
> Committer: Thomas Gleixner <tglx@linutronix.de>
> CommitterDate: Sun, 15 Nov 2020 20:47:14 +01:00
>
> timer_list: Use printk format instead of open-coded symbol lookup
>
> Use the "%ps" printk format string to resolve symbol names.
>
> This works on all platforms, including ia64, ppc64 and parisc64 on which
> one needs to dereference pointers to function descriptors instead of
> function pointers.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Link: https://lore.kernel.org/r/20201104163401.GA3984@ls3530.fritz.box
>
>
> ---
>  kernel/time/timer_list.c | 66 +++++++++++----------------------------
>  1 file changed, 19 insertions(+), 47 deletions(-)
>
> diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
> index acb326f..6939140 100644
> --- a/kernel/time/timer_list.c
> +++ b/kernel/time/timer_list.c
> @@ -42,24 +42,11 @@ static void SEQ_printf(struct seq_file *m, const char *fmt, ...)
>   va_end(args);
>  }
>
>
> -static void print_name_offset(struct seq_file *m, void *sym)
> -{
> - char symname[KSYM_NAME_LEN];
> -
> - if (lookup_symbol_name((unsigned long)sym, symname) < 0)
> - SEQ_printf(m, "<%pK>", sym);
> - else
> - SEQ_printf(m, "%s", symname);
> -}
> -
>  static void
>  print_timer(struct seq_file *m, struct hrtimer *taddr, struct hrtimer *timer,
>   int idx, u64 now)
>  {
> - SEQ_printf(m, " #%d: ", idx);
> - print_name_offset(m, taddr);
> - SEQ_printf(m, ", ");
> - print_name_offset(m, timer->function);
> + SEQ_printf(m, " #%d: <%pK>, %ps", idx, taddr, timer->function);
>   SEQ_printf(m, ", S:%02x", timer->state);
>   SEQ_printf(m, "\n");

trivia:

This could be coalesced into a single line statement.

SEQ_printf(m, "%d: <%pK>, %ps, S:%02x\n",
idx, taddr, timer->function, timer->state);

[]

> - if (dev->set_state_periodic) {
> - SEQ_printf(m, " periodic: ");
> - print_name_offset(m, dev->set_state_periodic);
> - SEQ_printf(m, "\n");
> - }
> + if (dev->set_state_periodic)
> + SEQ_printf(m, " periodic: %ps\n",
> + dev->set_state_periodic);

There is now additional whitespace after periodic: and oneshot:

This _might_ break some silly script that uses fixed column alignment
for the SEQ_ output.

It does look nicer now though.