Re: [PATCH bpf v2 3/3] tools/resolve_btfids: Skip unresolved symbol warning for empty BTF sets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 30, 2021 at 05:16:46AM IST, Andrii Nakryiko wrote:
> On Mon, Nov 22, 2021 at 6:47 AM Kumar Kartikeya Dwivedi
> <memxor@xxxxxxxxx> wrote:
> >
> > resolve_btfids prints a warning when it finds an unresolved symbol,
> > (id == 0) in id_patch. This can be the case for BTF sets that are empty
> > (due to disabled config options), hence printing warnings for certain
> > builds, most recently seen in [0].
> >
> > The reason behind this is because id->cnt aliases id->id in btf_id
>
> do we need to alias this, btw? We are trying to save 4 bytes on 800+
> struct (addr[ADDR_CNT] is big) and instead are getting more confusion
> in the code.
>

I can do that, but going to wait for Jiri's response before respinning.

> > struct, leading to empty set showing up as ID 0 when we get to id_patch,
> > which triggers the warning. Since sets are an exception here, accomodate
> > by reusing hole in btf_id for bool is_set member, setting it to true for
> > BTF set when setting id->cnt, and use that to skip extraneous warning.
> >
> >   [0]: https://lore.kernel.org/all/1b99ae14-abb4-d18f-cc6a-d7e523b25542@xxxxxxxxx
> >
> > Before:
> >
> > ; ./tools/bpf/resolve_btfids/resolve_btfids -v -b vmlinux net/ipv4/tcp_cubic.ko
> > adding symbol tcp_cubic_kfunc_ids
> > WARN: resolve_btfids: unresolved symbol tcp_cubic_kfunc_ids
> > patching addr     0: ID       0 [tcp_cubic_kfunc_ids]
> > sorting  addr     4: cnt      0 [tcp_cubic_kfunc_ids]
> > update ok for net/ipv4/tcp_cubic.ko
> >
> > After:
> >
> > ; ./tools/bpf/resolve_btfids/resolve_btfids -v -b vmlinux net/ipv4/tcp_cubic.ko
> > adding symbol tcp_cubic_kfunc_ids
> > patching addr     0: ID       0 [tcp_cubic_kfunc_ids]
> > sorting  addr     4: cnt      0 [tcp_cubic_kfunc_ids]
> > update ok for net/ipv4/tcp_cubic.ko
> >
> > Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
>
> Jiri, can you please take a look as well?
>
> > Fixes: 0e32dfc80bae ("bpf: Enable TCP congestion control kfunc from modules")
> > Reported-by: Pavel Skripkin <paskripkin@xxxxxxxxx>
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> > ---
> >  tools/bpf/resolve_btfids/main.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> > index a59cb0ee609c..73409e27be01 100644
> > --- a/tools/bpf/resolve_btfids/main.c
> > +++ b/tools/bpf/resolve_btfids/main.c
> > @@ -83,6 +83,7 @@ struct btf_id {
> >                 int      cnt;
> >         };
> >         int              addr_cnt;
> > +       bool             is_set;
> >         Elf64_Addr       addr[ADDR_CNT];
> >  };
> >
> > @@ -451,8 +452,10 @@ static int symbols_collect(struct object *obj)
> >                          * in symbol's size, together with 'cnt' field hence
> >                          * that - 1.
> >                          */
> > -                       if (id)
> > +                       if (id) {
> >                                 id->cnt = sym.st_size / sizeof(int) - 1;
> > +                               id->is_set = true;
> > +                       }
> >                 } else {
> >                         pr_err("FAILED unsupported prefix %s\n", prefix);
> >                         return -1;
> > @@ -568,9 +571,8 @@ static int id_patch(struct object *obj, struct btf_id *id)
> >         int *ptr = data->d_buf;
> >         int i;
> >
> > -       if (!id->id) {
> > +       if (!id->id && !id->is_set)
> >                 pr_err("WARN: resolve_btfids: unresolved symbol %s\n", id->name);
> > -       }
> >
> >         for (i = 0; i < id->addr_cnt; i++) {
> >                 unsigned long addr = id->addr[i];
> > --
> > 2.34.0
> >

--
Kartikeya



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux