On Mon, Mar 25, 2024 at 2:51 AM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote: > > On 23/03/2024 02:50, Alexei Starovoitov wrote: > > On Fri, Mar 22, 2024 at 3:26 AM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote: > >> > >> Support creation of module BTF along with base reference BTF; > >> the latter is stored in a .BTF.base_ref ELF section and supplements > >> split BTF references to base BTF with information about base types, > >> allowing for later reconciliation of split BTF with a (possibly > >> changed) base. resolve_btfids uses the "-r" option to specify > >> that the BTF.ids section should be populated with split BTF > >> relative to the added .BTF.base_ref section rather than relative > >> to the vmlinux base. > >> > >> Modules using base reference BTF can be built via > >> > >> BTF_BASE_REF=1 make -C. -M=path2/module > >> > >> The default is still to use split BTF relative to vmlinux. > >> > >> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx> > >> --- > >> scripts/Makefile.btf | 7 +++++++ > >> scripts/Makefile.modfinal | 4 ++-- > >> 2 files changed, 9 insertions(+), 2 deletions(-) > >> > >> diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf > >> index 9694ca3c5252..c8212b2ab7ca 100644 > >> --- a/scripts/Makefile.btf > >> +++ b/scripts/Makefile.btf > >> @@ -19,4 +19,11 @@ pahole-flags-$(call test-ge, $(pahole-ver), 126) = -j --btf_features=encode_forc > >> > >> pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE) += --lang_exclude=rust > >> > >> +ifeq ($(BTF_BASE_REF),1) > >> +module-pahole-flags-$(call test-ge, $(pahole-ver), 126) += --btf_features=base_ref > >> +module-resolve-btfids-flags-$(call test-ge, $(pahole-ver), 126) = -r > >> +endif > > > > The patch set looks great to me. > > I wonder whether we should drop this extra BTF_BASE_REF flag > > and do it unconditionally. > > Currently btf_parse_module() doesn't have any real checks > > whether module's btf matches base_btf. > > All the btf_check_*() might succeed by luck even if vmlinux btf > > is different. > > Since .BTF.base_ref is small we can always emit it and > > during module load the btf_reconcile() step will be that safety check. > > Less build options, less moving pieces and doing it all the time > > will make the whole process robust. > > Conditional BTF_BASE_REF=1 will likely mean that there will be > > bugs that only few folks will hit. > > We could certainly do it unconditionally for out-of-tree module builds > (the KBUILD_EXTMOD case); so anytime a user does "make -C. > M=path/2/module", they would get base reference BTF. With this series, > that would just mean applying this change: > > diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf > index c8212b2ab7ca..0322f8450a89 100644 > --- a/scripts/Makefile.btf > +++ b/scripts/Makefile.btf > @@ -19,7 +19,7 @@ pahole-flags-$(call test-ge, $(pahole-ver), 126) > = -j --btf_features=encode_forc > > pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE) += > --lang_exclude=rust > > -ifeq ($(BTF_BASE_REF),1) > +ifneq ($(KBUILD_EXTMOD),) > module-pahole-flags-$(call test-ge, $(pahole-ver), 126) += > --btf_features=base_ref > module-resolve-btfids-flags-$(call test-ge, $(pahole-ver), 126) = -r > endif > > So with this approach, in-tree modules don't add base reference BTF > since they don't need it, while out-of-tree module builds always do. > We could arrange for bpf_testmod to be built both ways perhaps to ensure > both approaches get tested in selftests along with kfunc addition etc. > > For adding base reference BTF in all cases, I ran the numbers on adding > base reference BTF across all kernel modules and for x86_64 with 2667 > modules. Base reference BTF adds around 4MB in total. Not huge, but for > in-tree builds, the only cases we've seen that triggered BTF mismatches > have been broken cases where vmlinux got built twice during kernel > build. So my suggestion would be to limit base reference BTF addition to > the KBUILD_EXTMOD case, and make it unconditional for that case only. So 4M across 2667 modules is ~1.5Kbyte per module ? And how many modules out of 2667 will be loaded? Even for 100 loaded modules it's just 150Kbyte of extra memory. But I'm fine with sticking to KBUILD_EXTMOD only. Let's make bpf_testmod with it though unconditionally. No need to build it twice.
