On Wed, Feb 09, 2022 at 10:04:18PM IST, Alexei Starovoitov wrote: > On Tue, Feb 8, 2022 at 10:22 PM Kumar Kartikeya Dwivedi > <memxor@xxxxxxxxx> wrote: > > > > On Wed, Feb 09, 2022 at 11:13:15AM IST, Alexei Starovoitov wrote: > > > From: Alexei Starovoitov <ast@xxxxxxxxxx> > > > > > > The main change is a move of the single line > > > #include "iterators.lskel.h" > > > from iterators/iterators.c to bpf_preload_kern.c. > > > Which means that generated light skeleton can be used from user space or > > > user mode driver like iterators.c or from the kernel module or the kernel itself. > > > The direct use of light skeleton from the kernel module simplifies the code, > > > since UMD is no longer necessary. The libbpf.a required user space and UMD. The > > > CO-RE in the kernel and generated "loader bpf program" used by the light > > > skeleton are capable to perform complex loading operations traditionally > > > provided by libbpf. In addition UMD approach was launching UMD process > > > every time bpffs has to be mounted. With light skeleton in the kernel > > > the bpf_preload kernel module loads bpf iterators once and pins them > > > multiple times into different bpffs mounts. > > > > > > Acked-by: Yonghong Song <yhs@xxxxxx> > > > Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx> > > > --- > > > kernel/bpf/inode.c | 39 ++---- > > > kernel/bpf/preload/Kconfig | 7 +- > > > kernel/bpf/preload/Makefile | 14 +-- > > > kernel/bpf/preload/bpf_preload.h | 8 +- > > > kernel/bpf/preload/bpf_preload_kern.c | 119 ++++++++---------- > > > kernel/bpf/preload/bpf_preload_umd_blob.S | 7 -- > > > .../preload/iterators/bpf_preload_common.h | 13 -- > > > kernel/bpf/preload/iterators/iterators.c | 108 ---------------- > > > kernel/bpf/syscall.c | 2 + > > > 9 files changed, 70 insertions(+), 247 deletions(-) > > > delete mode 100644 kernel/bpf/preload/bpf_preload_umd_blob.S > > > delete mode 100644 kernel/bpf/preload/iterators/bpf_preload_common.h > > > delete mode 100644 kernel/bpf/preload/iterators/iterators.c > > > > > > [...] > > > > > > -static int __init load_umd(void) > > > +static int __init load(void) > > > { > > > int err; > > > > > > - err = umd_load_blob(&umd_ops.info, &bpf_preload_umd_start, > > > - &bpf_preload_umd_end - &bpf_preload_umd_start); > > > + err = load_skel(); > > > if (err) > > > return err; > > > - bpf_preload_ops = &umd_ops; > > > + bpf_preload_ops = &ops; > > > return err; > > > } > > > > > > -static void __exit fini_umd(void) > > > +static void __exit fini(void) > > > { > > > - struct pid *tgid; > > > - > > > bpf_preload_ops = NULL; > > > - > > > - /* kill UMD in case it's still there due to earlier error */ > > > - tgid = umd_ops.info.tgid; > > > - if (tgid) { > > > - kill_pid(tgid, SIGKILL, 1); > > > - > > > - wait_event(tgid->wait_pidfd, thread_group_exited(tgid)); > > > - umd_cleanup_helper(&umd_ops.info); > > > - } > > > - umd_unload_blob(&umd_ops.info); > > > + free_links_and_skel(); > > > } > > > -late_initcall(load_umd); > > > -module_exit(fini_umd); > > > +late_initcall(load); > > > > Since load_skel invokes the loader which resolve kfuncs, and if the module is > > builtin, btf_vmlinux's kfunc_set_tab is not fully populated yet (other > > register_btf_id_kfunc_set calls may also happen in late_initcall), so I think it > > may be a problem. > > > > When I worked on it we didn't have this case where BPF syscall can be invoked at > > this point, but obviously that is true now. So I think ordering it later than > > late_initcall (or moving register_btf_id_kfunc_set invoking initcalls before) is > > required. WDYT? > > Good point. Probably best to convert register_btf_kfunc_id_set invocations > to core_initcall. > I can follow up with a trivial patch later or you can beat me to it. Cool, I'll post it after this is merged. -- Kartikeya