On Thu, 2023-02-09 at 09:47 +0100, Jiri Olsa wrote: > On Mon, Feb 06, 2023 at 10:15:37AM +0100, Jiri Olsa wrote: > > On Sun, Feb 05, 2023 at 07:36:14PM +0100, Ilya Leoshkevich wrote: > > > On Sun, 2023-02-05 at 19:17 +0100, Jiri Olsa wrote: > > > > On Sat, Feb 04, 2023 at 01:21:13AM -0800, Alexei Starovoitov > > > > wrote: > > > > > On Fri, Feb 3, 2023 at 8:23 AM Jiri Olsa <jolsa@xxxxxxxxxx> > > > > > wrote: > > > > > > > > > > > > hi, > > > > > > I noticed several times in discussions that we should move > > > > > > test > > > > > > kfuncs > > > > > > into kernel module, now perhaps even more pressing with all > > > > > > the > > > > > > kfunc > > > > > > effort. This patchset moves all the test kfuncs into > > > > > > bpf_testmod. > > > > > > > > > > > > I added bpf_testmod/bpf_testmod_kfunc.h header that is > > > > > > shared > > > > > > between > > > > > > bpf_testmod kernel module and BPF programs, which brings > > > > > > some > > > > > > difficulties > > > > > > with __ksym define. But I'm not sure having separate > > > > > > headers for > > > > > > BPF > > > > > > programs and for kernel module would be better. > > > > > > > > > > > > This patchset also needs: > > > > > > 74bc3a5acc82 bpf: Add missing btf_put to > > > > > > register_btf_id_dtor_kfuncs > > > > > > which is only in bpf/master now. > > > > > > > > > > I thought you've added this patch to CI, > > > > > but cb_refs is still failing on s390... > > > > > > > > the CI now fails for s390 with messages like: > > > > 2023-02-04T07:04:32.5185267Z RES: address of kernel > > > > function > > > > bpf_kfunc_call_test_fail1 is out of range > > > > > > > > so now that we have test kfuncs in the module, the 's32 imm' > > > > value of > > > > the bpf call instructions can overflow when the offset between > > > > module > > > > and kernel is greater than 2GB ... as explained in the commit > > > > that > > > > added the verifier check: > > > > > > > > 8cbf062a250e bpf: Reject kfunc calls that overflow insn->imm > > > > > > > > not sure we can do anything about that on bpf side.. cc-ing > > > > s390 list > > > > and Ilya for ideas/thoughts > > > > > > > > maybe we could make bpf_testmod in-tree module and compile it > > > > as > > > > module > > > > just for some archs > > > > > > > > thoughts? > > > > > > Hi, > > > > > > I'd rather have this fixed - I guess the problem can affect the > > > users. > > > The ksyms_module test is already denylisted because of that. > > > Unfortunately getting the kernel and the modules close together > > > on > > > s390x is unlikely to happen in the foreseeable future. > > > > > > What do you think about keeping the BTF ID inside the insn->imm > > > field > > > and putting the 64-bit delta into bpf_insn_aux_data, replacing > > > the > > > call_imm field that we already have there? > > > > seems tricky wrt other archs.. how about saving address of the > > kfunc > > in bpf_insn_aux_data and use that in s390 jit code instead of the > > '__bpf_call_base + imm' calculation > > any other ideas/thoughts on this? > > I don't have s390 server available, so can't really fix/test that.. > any chance you work on that? Hi Jiri, sure, I'll give this a try. Best regards, Ilya > > thanks, > jirka