Re: [PATCH bpf-next 1/2] tools/resolve_btfids: emit warnings and patch zero id for missing symbols

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

 





On 7/25/21 10:22 PM, Hengqi Chen wrote:


On 2021/7/26 12:56 PM, Yonghong Song wrote:


On 7/25/21 9:41 PM, Hengqi Chen wrote:


On 2021/7/26 11:32 AM, Yonghong Song wrote:


On 7/25/21 7:18 AM, Hengqi Chen wrote:
Kernel functions referenced by .BTF_ids may changed from global to static
and get inlined and thus disappears from BTF. This causes kernel build

the function could be renamed or removed too.

failure when resolve_btfids do id patch for symbols in .BTF_ids in vmlinux.
Update resolve_btfids to emit warning messages and patch zero id for missing
symbols instead of aborting kernel build process.

Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx>

LGTM with one minor comment below.

Acked-by: Yonghong Song <yhs@xxxxxx>

---
    tools/bpf/resolve_btfids/main.c | 13 +++++++------
    1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index 3ad9301b0f00..3ea19e33250d 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -291,7 +291,7 @@ static int compressed_section_fix(Elf *elf, Elf_Scn *scn, GElf_Shdr *sh)
        sh->sh_addralign = expected;

        if (gelf_update_shdr(scn, sh) == 0) {
-        printf("FAILED cannot update section header: %s\n",
+        pr_err("FAILED cannot update section header: %s\n",
                elf_errmsg(-1));
            return -1;
        }
@@ -317,6 +317,7 @@ static int elf_collect(struct object *obj)

        elf = elf_begin(fd, ELF_C_RDWR_MMAP, NULL);
        if (!elf) {
+        close(fd);
            pr_err("FAILED cannot create ELF descriptor: %s\n",
                elf_errmsg(-1));
            return -1;
@@ -484,7 +485,7 @@ static int symbols_resolve(struct object *obj)
        err = libbpf_get_error(btf);
        if (err) {
            pr_err("FAILED: load BTF from %s: %s\n",
-            obj->path, strerror(-err));
+            obj->btf ?: obj->path, strerror(-err));

Why you change "obj->path" to "obj->btf ?: obj->path"?
Note that obj->path cannot be NULL.

The diff didn't see the whole picture. Let me quote it here:
```
btf = btf__parse(obj->btf ?: obj->path, NULL);
err = libbpf_get_error(btf);
if (err) {
          pr_err("FAILED: load BTF from %s: %s\n",
                  obj->path, strerror(-err));
          return -1;
}
```

Because btf__parse parses either obj->btf or obj->path,
I think the error message should reveal this.

Okay, I see, obj->btf may not be NULL due to
                 OPT_STRING(0, "btf", &obj.btf, "BTF data",
                            "BTF data"),

How about
   obj->btf ? "input BTF data" : obj->path

The error message like
   FAILED: load BTF from : <error msg>
does not sound good.


Sorry, I am confused.

If obj->btf is set, say, vmlinux.btf, the message should look like:
  FAILED: load BTF from vmlinux.btf: <error msg>

Otherwise, it should look like:
  FAILED: load BTF from vmlinux: <error msg>

Am I missing something ?

Ah, you are right. Your patch looks good. I didn't pay attention
and thought
  OPT_STRING(0, "btf", &obj.btf, "BTF data","BTF data")
is a string input for *btf data*, but actually it is actually
/sys/kernel/btf/vmlinux.




            return -1;
        }

[...]



[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