On 3/21/24 5:17 PM, Andrii Nakryiko wrote:
On Thu, Mar 21, 2024 at 5:02 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
On Thu, Mar 21, 2024 at 4:55 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
static int kallsyms_cb(unsigned long long sym_addr, char sym_type,
const char *sym_name, void *ctx)
{
+ int lto_enabled = check_lto_kernel();
+ char orig_name[PATH_MAX], *res;
struct bpf_object *obj = ctx;
const struct btf_type *t;
struct extern_desc *ext;
- ext = find_extern_by_name(obj, sym_name);
+ /* Only check static variables in data sections */
+ if (sym_type == 'd' && obj->need_kallsyms && lto_enabled == 1) {
why bother grepping config.gz ?
I see no harm in doing below strstr unconditionally.
Do you mean we skip condition
sym_type == 'd' && obj->need_kallsyms && lto_enabled == 1
all together?
For condition sym_type == 'd', Andrii suggested (in private discussion)
to focus on data first since that is the issue we hitted. Of course
we could do all symbols here too.
For condition obj->need_kallsyms, I can remove this one.
For lto_enabled == 1, the main goal is to avoid extra overhead for
not-lto kernels.
I guess that the overhead is not that bad since typically symbol name
is not long. So removing all conditions seems indeed a viable solution.
I was suggesting to remove the last lto_enabled == 1 check and
check_lto_kernel() since it will simplify the patch significantly and
won't cause any slowdown.
+1, grepping Kconfig seems like an overkill
The first two checks... I'm not sure.
I'd say we shouldn't do this for functions, because if LLVM rewrites
them, then usually that means that function signature is changed,
which seems dangerous to just silently resolve. I'd start with data
symbols for now.
For this particular suffix '.llvm.<hash>', function signature will
not change. But considering all funcitons annotated with __ksym are
kfunc's and I cannot see currently how kfunc's could be cross-file
inlined.
It might be possible a kernel function is attached with __ksym as
bpf program wants to know the address of that kernel function.
But this should be a really corner case.
So agree and let me keep sym_type == 'd' condition only.
The less corner cases the better. strstr is fast enough.
yep, I doubt it would be noticeable if we do strstr()
Ok to remove them all.
