Re: [RFC PATCH bpf-next v2 2/2] selftests/bpf: add tests for selective kmod BTF loading
Fuyu Zhao <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 8/13/2026 11:37 AM, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] The tracking counter `obj->kmod_btfs->nr_loaded` is never updated, rendering the early termination optimization for BTF loading completely ineffective. > -- > > --- Patch [2]: [RFC PATCH bpf-next v2 2/2] selftests/bpf: add tests for selective kmod BTF loading --- > commit 506359525a106f517e013e060ee2c6feb17720c8 > Author: Fuyu Zhao <[email protected]> > > selftests/bpf: add tests for selective kmod BTF loading > > Add test cases to verify the new ".kmod_btfs" section logic in libbpf. > The added test cases cover three main scenarios: valid module, non-existent > module, and duplicates/unneeded modules. > >> diff --git a/tools/testing/selftests/bpf/prog_tests/kmod_btfs.c b/tools/testing/selftests/bpf/prog_tests/kmod_btfs.c >> index 0000000000000..d875fcce6681d >> --- /dev/null >> +++ b/tools/testing/selftests/bpf/prog_tests/kmod_btfs.c >> @@ -0,0 +1,52 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +#include <test_progs.h> >> +#include "kmod_btfs.skel.h" >> +#include "kmod_btfs_nonexist.skel.h" >> +#include "kmod_btfs_mix.skel.h" >> + > > [ ... ] > > [Severity: Medium] > While reviewing these tests, I noticed a potential regression in the libbpf > code they exercise. In load_module_btfs(): > > tools/lib/bpf/libbpf.c:load_module_btfs() { > ... > if (obj->kmod_btfs && > obj->kmod_btfs->nr_names == obj->kmod_btfs->nr_loaded) > break; > ... > } > > Is obj->kmod_btfs->nr_loaded ever incremented when a module BTF is > successfully loaded? > > If it is not incremented, will this early termination condition ever be > met? It appears this might unnecessarily iterate over all remaining > system BTFs, which would bypass the intended performance optimization > of the .kmod_btfs section. > Thanks for catching this. This is the same oversight mentioned above. I'll fix the nr_loaded accounting in v3.