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 ?
>>
>>>
>>>> return -1;
>>>> }
>>>>
>>> [...]
