On Sun, Feb 9, 2025 at 11:57 AM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Sat, Feb 8, 2025 at 11:32 AM Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote: > > > > On Sat, Feb 08, 2025 at 07:47:12AM -0800, Alexei Starovoitov wrote: > > > On Fri, Feb 7, 2025 at 10:42 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote: > > > > > > > > On Fri, Feb 7, 2025 at 2:01 AM Song Liu <song@xxxxxxxxxx> wrote: > > > > > > > > > > On Wed, Feb 5, 2025 at 6:55 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote: > > > > > [...] > > > > > > > I think we should first understand why the trampoline is not > > > > > > > freed. > > > > > > > > > > > > IIUC, the fexit works as follows, > > > > > > > > > > > > bpf_trampoline > > > > > > + __bpf_tramp_enter > > > > > > + percpu_ref_get(&tr->pcref); > > > > > > > > > > > > + call do_exit() > > > > > > > > > > > > + __bpf_tramp_exit > > > > > > + percpu_ref_put(&tr->pcref); > > > > > > > > > > > > Since do_exit() never returns, the refcnt of the trampoline image is > > > > > > never decremented, preventing it from being freed. > > > > > > > > > > Thanks for the explanation. In this case, I think it makes sense to > > > > > disallow attaching fexit programs on __noreturn functions. I am not > > > > > sure what is the best solution for it though. > > > > > > > > There is a tools/objtool/noreturns.h. Perhaps we could create a > > > > similar noreturns.h under kernel/bpf and add all relevant functions to > > > > the fexit deny list. > > > > > > Pls avoid copy paste if possible. > > > Something like: > > > > > > BTF_SET_START(fexit_deny) > > > #define NORETURN(fn) BTF_ID(func, fn) > > > #include "../../tools/objtool/noreturns.h" > > > > > > Should work? > > > > > > Josh, > > > maybe we should move noreturns.h to some common location? > > > > The tools code is meant to be independent from the kernel, but it could > > be synced by copying it to both include/linux and tools/include/linux, > > and then make sure it stays in sync with tools/objtool/sync-check.sh. > > > > However, noreturns.h is manually edited, and only for some arches. And > > even for those arches it's likely not exhaustive: we only add to it when > > we notice an objtool warning, and not all calls to noreturns will > > necessarily trigger a warning. So I'd be careful about relying on that. > > > > Also that file is intended to be temporary, there have been proposals to > > add compiler support for annotating noreturns. That hasn't been > > implemented yet, help wanted! > > > > I think the noreturn info is available in DWARF, can that be converted > > to BTF? > > There are 30k+ noreturn funcs in dwarf. So pahole would need to have > some heuristic to filter out the noise. > It's doable, but we need to stop the bleeding the simplest way > and the fix would need to be backported too. > We can copy paste noreturns.h or #include it from the current location > for now and think of better ways for -next. It seems like the simplest way at present. I will send a patch. -- Regards Yafang
