Thanks for the review, Alan. v2 is out and addresses these: https://lore.kernel.org/bpf/[email protected]/
Answers inline. > I have a question about the approach here; my experience with using sysfs > with a dummy placeholder file which triggered on-demand module load when > accessed was it didn't work with sysfs interfaces because the file size needed > to be refreshed, and the reader would usually error out since it saw an > empty file and gave up reading. I wound up having to switch to using > kernfs to update size attributes on open such that the caller would see the > size change synchronously once the load completed. See patch 15 in the above > series for the details. > > It seems like you used a different approach here, and by storing the size > in the BTF metadata this problem was avoided. From the below it seems like > there are no first-caller issues from userspace (like if the first caller > does "bpftool btf dump file /sys/kernel/btf/vmlinux")? Right, that is the reason for .BTF.meta: the size is known at build time, so the file is created with its final size at boot and the load happens in read()/mmap(). No kernfs internals, no i_size update. I have now tested exactly that with the in-tree bpftool (libbpf 1.8) as the first user on a fresh boot, nothing else having touched the BTF: bpftool btf dump file /sys/kernel/btf/vmlinux bpftool btf list Both work and lsmod shows btf_vmlinux afterwards. Same for libbpf-loaded programs as the first user: a global subprogram taking the context, bpf_snprintf_btf(), a CO-RE field read, and a kfunc of an out-of-tree module (below). > Another issue; during boot, request_module can call back out to modprobe > and depending on where you are in the boot process, the module may not be > available due to filesystem not mounted yet etc. Maybe this just means that > btf_vmlinux.ko needs to be in the initramfs image? If that's the case, I > would suggest highlighting that in the CONFIG_DEBUG_INFO_BTF Kconfig > description, Yes. If request_module() fails the caller gets NULL, i.e. behaves as on a kernel without BTF, and the next user retries; nothing is cached. So a program that needs kernel types before the root fs is mounted fails unless btf_vmlinux.ko is in the initramfs. I have added a paragraph along the lines you suggest to the Kconfig help in v2. > Another concern to balance; embedded folks were interested in vmlinux > BTF as a module to limit on-disk footprint rather than (or likely as > well as) runtime memory; [...] If I'm following, the final vmlinux image that > winds > up on disk doesn't contain the BTF section, right? If so that's great > news for them. Right. With =m v2 strips the BTF from vmlinux, so no boot image carries it; it only lives in btf_vmlinux.ko, which a system can also choose not to install. > And another thing I was wondering about - did you test with modules > containing a .BTF.base (built standalone via "make -C path2module")? Not in v1, thanks for asking. Looking at that case showed a problem, fixed in v2: for a module with .BTF.base the raw .BTF is only valid against the distilled base and is rewritten in place when it is relocated, so it must not be served before that. In v2 the sysfs file of such a module is still created at load time with its final size (the relocation only rewrites ids and string offsets), but its reader loads the vmlinux BTF and waits until this module's BTF is relocated and published before serving it; modules without .BTF.base, whose data is final, are served as before. Tested with an out-of-tree module that has a .BTF.base and registers a kfunc from its init, loaded before the vmlinux BTF: once the BTF is loaded it gets its id, relocated types and its kfunc, and a program calling that kfunc works. > What I meant by the hard part (aside from deferred handling which is hard > enough!) is that there is a conflict between the goal of saving memory and > being > forced to allocate memory for module BTF for a deferred-load scheme like this. > [...] > CONFIG_DEBUG_INFO_BTF=y results in upfront allocation of memory for kernel > and module BTF > CONFIG_DEBUG_INFO_BTF=m results in upfront allocation of memory for module > BTF only Agreed, and v2 says so explicitly in the Kconfig help and the cover letter: the copy kept for a deferred module is the same copy btf_parse_module() makes with =y, so module BTF costs the same at the same time in both; the saving is the vmlinux BTF only. > It might be worth thinking about providing a means to control whether such > module allocations happen prior to vmlinux BTF loading for highly > memory-constrained > systems. Anything delivering kfuncs etc should probably always allocate since > it > constitutes core BPF infrastructure. Makes sense as a follow-up; I would rather not grow this series with it. The distinction you draw (keep it for modules that register kfuncs or struct_ops, make it optional otherwise) is what I would start from. Thanks, Jay
