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.
The first two checks... I'm not sure.
The less corner cases the better. strstr is fast enough.
Ok to remove them all.
