[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALOAHbA3b8GOxKn9FkbiWEBDkXJ+kY=pVkYMruQyki_0K=1DtQ@mail.gmail.com>
Date: Mon, 10 Feb 2025 10:39:20 +0800
From: Yafang Shao <laoar.shao@...il.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Josh Poimboeuf <jpoimboe@...nel.org>, Song Liu <song@...nel.org>, bpf <bpf@...r.kernel.org>,
Miroslav Benes <mbenes@...e.cz>, Petr Mladek <pmladek@...e.com>, Jiri Kosina <jikos@...nel.org>,
Joe Lawrence <joe.lawrence@...hat.com>, live-patching@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 0/2] livepatch: Add support for hybrid mode
On Sun, Feb 9, 2025 at 11:57 AM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Sat, Feb 8, 2025 at 11:32 AM Josh Poimboeuf <jpoimboe@...nel.org> 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@...il.com> wrote:
> > > >
> > > > On Fri, Feb 7, 2025 at 2:01 AM Song Liu <song@...nel.org> wrote:
> > > > >
> > > > > On Wed, Feb 5, 2025 at 6:55 PM Yafang Shao <laoar.shao@...il.com> 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
Powered by blists - more mailing lists