Re: [PATCH bpf-next 2/2] bpf: Show total linked progs cnt instead of selector in trampoline ksym

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, May 10, 2023 at 11:33:21PM +0800, Yafang Shao wrote:
> On Wed, May 10, 2023 at 2:30 PM Song Liu <song@kernel.org> wrote:
> >
> > On Tue, May 9, 2023 at 7:56 PM Yafang Shao <laoar.shao@gmail.com> wrote:
> > >
> > > On Wed, May 10, 2023 at 1:43 AM Song Liu <song@kernel.org> wrote:
> > > >
> > > > On Tue, May 9, 2023 at 8:15 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > > > >
> > > > > After commit e21aa341785c ("bpf: Fix fexit trampoline."), the selector
> > > > > is only used to indicate how many times the bpf trampoline image are
> > > > > updated and been displayed in the trampoline ksym name. After the
> > > > > trampoline is freed, the count will start from 0 again.
> > > > > So the count is a useless value to the user, we'd better
> > > > > show a more meaningful value like how many progs are linked to this
> > > > > trampoline. After that change, the selector can be removed eventally.
> > > > > If the user want to check whether the bpf trampoline image has been updated
> > > > > or not, the user can also compare the address. Each time the trampoline
> > > > > image is updated, the address will change consequently.
> > > >
> > > > I wonder whether this will cause confusion to some users. Maybe the saving
> > > > doesn't worth the churn.
> > >
> > > The trampoline ksym name as such:
> > > ffffffffc06c3000 t bpf_trampoline_6442453466_1  [bpf]
> > >
> > > I don't know what the user may use the selector for. It seems that the
> > > selector is meaningless. While the cnt of linked progs can really help
> > > users, with which the user can easily figure out how many progs are
> > > linked to a kernel function.
> >
> > Hmm, agreed that the chance to break user space is low. Maybe we can just
> > remove it? IOW, only keep bpf_trampoline_6442453466
> >
> 
> Agree. I will do it as you suggested.

perf is actually is still trying parse the old name

        /* .. and only for trampolines and dispatchers */
        if ((sscanf(name, "bpf_trampoline_%lu", &id) == 1) ||
            (sscanf(name, "bpf_dispatcher_%s", disp) == 1))
                err = process_bpf_image(name, start, data);

so this change would actualy fix it ;-)

thanks,
jirka