[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YmJPcU9dahEatb0f@krava>
Date: Fri, 22 Apr 2022 08:47:13 +0200
From: Jiri Olsa <olsajiri@...il.com>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, netdev@...r.kernel.org,
bpf@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...omium.org>
Subject: Re: [PATCHv2 bpf-next 1/4] kallsyms: Add kallsyms_lookup_names
function
On Tue, Apr 19, 2022 at 10:26:05AM +0200, Jiri Olsa wrote:
SNIP
> > > +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;
> >
> > Ooops, wait. Did you do this last version? I missed this point.
> > This changes the meanings of the kernel function.
>
> yes, it was there before ;-) and you're right.. so some archs can
> return different address, I did not realize that
>
> >
> > > +
> > > + args->addrs[args->found++] = addr;
> > > + return args->found == args->cnt ? 1 : 0;
> > > +}
> > > +
> > > +/**
> > > + * kallsyms_lookup_names - Lookup addresses for array of symbols
> >
> > More correctly "Lookup 'ftraced' addresses for array of sorted symbols", right?
> >
> > I'm not sure, we can call it as a 'kallsyms' API, since this is using
> > kallsyms but doesn't return symbol address, but ftrace address.
> > I think this name misleads user to expect returning symbol address.
> >
> > > + *
> > > + * @syms: array of symbols pointers symbols to resolve, must be
> > > + * alphabetically sorted
> > > + * @cnt: number of symbols/addresses in @syms/@...rs 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.
> >
> > Hmm, sorry I changed my mind. I rather like to expose kallsyms_on_each_symbol()
> > and provide this API from fprobe or ftrace, because this returns ftrace address
> > and thus this is only used from fprobe.
>
> ok, so how about:
>
> int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs);
quick question.. is it ok if it stays in kalsyms.c object?
so we don't need to expose kallsyms_on_each_symbol,
and it stays in 'kalsyms' place
jirka
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index ce1bd2fbf23e..177e0b13c8c5 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -72,6 +72,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
#ifdef CONFIG_KALLSYMS
/* Lookup the address for a symbol. Returns 0 if not found. */
unsigned long kallsyms_lookup_name(const char *name);
+int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs);
extern int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
@@ -103,6 +104,11 @@ static inline unsigned long kallsyms_lookup_name(const char *name)
return 0;
}
+static inline int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs);
+{
+ return -ERANGE;
+}
+
static inline int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 79f2eb617a62..1e7136a765a9 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
@@ -228,7 +229,7 @@ unsigned long kallsyms_lookup_name(const char *name)
return module_kallsyms_lookup_name(name);
}
-#ifdef CONFIG_LIVEPATCH
+#if defined(CONFIG_LIVEPATCH) || defined(CONFIG_FPROBE)
/*
* Iterate over all symbols in vmlinux. For symbols from modules use
* module_kallsyms_on_each_symbol instead.
@@ -572,6 +573,73 @@ int sprint_backtrace_build_id(char *buffer, unsigned long address)
return __sprint_symbol(buffer, address, -1, 1, 1);
}
+#ifdef CONFIG_FPROBE
+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/@...rs 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;
+
+ args.addrs = addrs;
+ args.syms = sorted_syms;
+ args.cnt = cnt;
+ args.found = 0;
+ kallsyms_on_each_symbol(kallsyms_callback, &args);
+
+ return args.found == args.cnt ? 0 : -ESRCH;
+}
+#else
+int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs)
+{
+ return -ERANGE;
+}
+#endif /* CONFIG_FPROBE */
+
/* To avoid using get_symbol_offset for every symbol, we carry prefix along. */
struct kallsym_iter {
loff_t pos;
Powered by blists - more mailing lists