Re: [PATCHv4 bpf-next 2/5] ftrace: Add ftrace_lookup_symbols function

Masami Hiramatsu
2022-04-29T14:56:28Z

On Thu, 28 Apr 2022 22:12:04 +0200
Jiri Olsa <jolsa@kernel.org> wrote:

> Adding ftrace_lookup_symbols function that resolves array of symbols
> with single pass over kallsyms.
>
> The user provides array of string pointers with count and pointer to
> allocated array for resolved values.
>
> int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt,
> unsigned long *addrs)
>
> It iterates all kallsyms symbols and tries to loop up each in provided
> symbols array with bsearch. The symbols array needs to be sorted by
> name for this reason.
>
> We also check each symbol to pass ftrace_location, because this API
> will be used for fprobe symbols resolving.

This looks good to me.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> include/linux/ftrace.h | 6 ++++
> kernel/kallsyms.c | 1 +
> kernel/trace/ftrace.c | 62 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 69 insertions(+)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 4816b7e11047..820500430eae 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -303,6 +303,8 @@ int unregister_ftrace_function(struct ftrace_ops *ops);
> extern void ftrace_stub(unsigned long a0, unsigned long a1,
> struct ftrace_ops *op, struct ftrace_regs *fregs);
>
> +
> +int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs);
> #else /* !CONFIG_FUNCTION_TRACER */
> /*
> * (un)register_ftrace_function must be a macro since the ops parameter
> @@ -313,6 +315,10 @@ extern void ftrace_stub(unsigned long a0, unsigned long a1,
> static inline void ftrace_kill(void) { }
> static inline void ftrace_free_init_mem(void) { }
> static inline void ftrace_free_mem(struct module *mod, void *start, void *end) { }
> +static inline int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif /* CONFIG_FUNCTION_TRACER */
>
> struct ftrace_func_entry {
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index fdfd308bebc4..fbdf8d3279ac 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -29,6 +29,7 @@
> #include <linux/compiler.h>
> #include <linux/module.h>
> #include <linux/kernel.h>
> +#include <linux/bsearch.h>
>
> /*
> * These will be re-linked against their real values
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 4f1d2f5e7263..07d87c7a525d 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -7964,3 +7964,65 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
> mutex_unlock(&ftrace_lock);
> return ret;
> }
> +
> +static int symbols_cmp(const void *a, const void *b)
> +{
> + const char **str_a = (const char **) a;
> + const char **str_b = (const char **) b;
> +
> + return strcmp(*str_a, *str_b);
> +}
> +
> +struct kallsyms_data {
> + unsigned long *addrs;
> + const char **syms;
> + size_t cnt;
> + size_t found;
> +};
> +
> +static int kallsyms_callback(void *data, const char *name,
> + struct module *mod, unsigned long addr)
> +{
> + struct kallsyms_data *args = data;
> +
> + if (!bsearch(&name, args->syms, args->cnt, sizeof(*args->syms), symbols_cmp))
> + return 0;
> +
> + addr = ftrace_location(addr);
> + if (!addr)
> + return 0;
> +
> + args->addrs[args->found++] = addr;
> + return args->found == args->cnt ? 1 : 0;
> +}
> +
> +/**
> + * ftrace_lookup_symbols - Lookup addresses for array of symbols
> + *
> + * @sorted_syms: array of symbols pointers symbols to resolve,
> + * must be alphabetically sorted
> + * @cnt: number of symbols/addresses in @syms/@addrs arrays
> + * @addrs: array for storing resulting addresses
> + *
> + * This function looks up addresses for array of symbols provided in
> + * @syms array (must be alphabetically sorted) and stores them in
> + * @addrs array, which needs to be big enough to store at least @cnt
> + * addresses.
> + *
> + * This function returns 0 if all provided symbols are found,
> + * -ESRCH otherwise.
> + */
> +int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs)
> +{
> + struct kallsyms_data args;
> + int err;
> +
> + args.addrs = addrs;
> + args.syms = sorted_syms;
> + args.cnt = cnt;
> + args.found = 0;
> + err = kallsyms_on_each_symbol(kallsyms_callback, &args);
> + if (err < 0)
> + return err;
> + return args.found == args.cnt ? 0 : -ESRCH;
> +}
> --
> 2.35.1
>

--
Masami Hiramatsu <mhiramat@kernel.org>