| Message ID | 20220510074659.2557731-1-jolsa@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | perf tools: Fix prologue generation | expand |
On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote: > > hi, > sending change we discussed some time ago [1] to get rid of > some deprecated functions we use in perf prologue code. > > Despite the gloomy discussion I think the final code does > not look that bad ;-) > > This patchset removes following libbpf functions from perf: > bpf_program__set_prep > bpf_program__nth_fd > struct bpf_prog_prep_result > > v2 changes: > - use fallback section prog handler, so we don't need to > use section prefix [Andrii] > - realloc prog->insns array in bpf_program__set_insns [Andrii] > - squash patch 1 from previous version with > bpf_program__set_insns change [Daniel] > - patch 3 already merged [Arnaldo] > - added more comments > > meanwhile.. perf/core and bpf-next diverged, so: > - libbpf bpf_program__set_insns change is based on bpf-next/master > - perf changes do not apply on bpf-next/master so they are based on > perf/core ... however they can be merged only after we release > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > the dynamic linking > I'm sending perf changes now just for review, I'll resend them > once libbpf 0.8.0 is released > > thanks, > jirka > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > --- > Jiri Olsa (1): > libbpf: Add bpf_program__set_insns function > The first patch looks good to me. The rest I can't really review and test properly, so I'll leave it up to Arnaldo. Arnaldo, how do we coordinate these patches? Should they go through bpf-next (after you Ack them) or you want them in your tree? I'd like to get the bpf_program__set_insns() patch into bpf-next so that I can do libbpf v0.8 release, having it in a separate tree is extremely inconvenient. Please let me know how you think we should proceed? > tools/lib/bpf/libbpf.c | 22 ++++++++++++++++++++++ > tools/lib/bpf/libbpf.h | 18 ++++++++++++++++++ > tools/lib/bpf/libbpf.map | 1 + > 3 files changed, 41 insertions(+) > > Jiri Olsa (2): > perf tools: Register fallback libbpf section handler > perf tools: Rework prologue generation code > > tools/perf/util/bpf-loader.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 157 insertions(+), 18 deletions(-)
On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote: > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > hi, > > sending change we discussed some time ago [1] to get rid of > > some deprecated functions we use in perf prologue code. > > > > Despite the gloomy discussion I think the final code does > > not look that bad ;-) > > > > This patchset removes following libbpf functions from perf: > > bpf_program__set_prep > > bpf_program__nth_fd > > struct bpf_prog_prep_result > > > > v2 changes: > > - use fallback section prog handler, so we don't need to > > use section prefix [Andrii] > > - realloc prog->insns array in bpf_program__set_insns [Andrii] > > - squash patch 1 from previous version with > > bpf_program__set_insns change [Daniel] > > - patch 3 already merged [Arnaldo] > > - added more comments > > > > meanwhile.. perf/core and bpf-next diverged, so: > > - libbpf bpf_program__set_insns change is based on bpf-next/master > > - perf changes do not apply on bpf-next/master so they are based on > > perf/core ... however they can be merged only after we release > > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > > the dynamic linking > > I'm sending perf changes now just for review, I'll resend them > > once libbpf 0.8.0 is released > > > > thanks, > > jirka > > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > > --- > > Jiri Olsa (1): > > libbpf: Add bpf_program__set_insns function > > > > The first patch looks good to me. The rest I can't really review and > test properly, so I'll leave it up to Arnaldo. > > Arnaldo, how do we coordinate these patches? Should they go through > bpf-next (after you Ack them) or you want them in your tree? > > I'd like to get the bpf_program__set_insns() patch into bpf-next so > that I can do libbpf v0.8 release, having it in a separate tree is > extremely inconvenient. Please let me know how you think we should > proceed? we need to wait with perf changes after the libbpf is merged and libbpf 0.8.0 is released.. so we don't break dynamic linking for perf at the moment please just take libbpf change and I'll resend the perf change later if needed thanks, jirka
Hello: This series was applied to bpf/bpf-next.git (master) by Daniel Borkmann <daniel@iogearbox.net>: On Tue, 10 May 2022 09:46:56 +0200 you wrote: > hi, > sending change we discussed some time ago [1] to get rid of > some deprecated functions we use in perf prologue code. > > Despite the gloomy discussion I think the final code does > not look that bad ;-) > > [...] Here is the summary with links: - [PATCHv2,bpf-next,1/3] libbpf: Add bpf_program__set_insns function https://git.kernel.org/bpf/bpf-next/c/b63b3c490eee - [PATCHv2,perf/core,2/3] perf tools: Register fallback libbpf section handler (no matching commit) - [PATCHv2,perf/core,3/3] perf tools: Rework prologue generation code (no matching commit) You are awesome, thank you!
Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu: > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote: > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > hi, > > > sending change we discussed some time ago [1] to get rid of > > > some deprecated functions we use in perf prologue code. > > > > > > Despite the gloomy discussion I think the final code does > > > not look that bad ;-) > > > > > > This patchset removes following libbpf functions from perf: > > > bpf_program__set_prep > > > bpf_program__nth_fd > > > struct bpf_prog_prep_result > > > > > > v2 changes: > > > - use fallback section prog handler, so we don't need to > > > use section prefix [Andrii] > > > - realloc prog->insns array in bpf_program__set_insns [Andrii] > > > - squash patch 1 from previous version with > > > bpf_program__set_insns change [Daniel] > > > - patch 3 already merged [Arnaldo] > > > - added more comments > > > > > > meanwhile.. perf/core and bpf-next diverged, so: > > > - libbpf bpf_program__set_insns change is based on bpf-next/master > > > - perf changes do not apply on bpf-next/master so they are based on > > > perf/core ... however they can be merged only after we release > > > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > > > the dynamic linking > > > I'm sending perf changes now just for review, I'll resend them > > > once libbpf 0.8.0 is released > > > > > > thanks, > > > jirka > > > > > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > > > --- > > > Jiri Olsa (1): > > > libbpf: Add bpf_program__set_insns function > > > > > > > The first patch looks good to me. The rest I can't really review and > > test properly, so I'll leave it up to Arnaldo. > > > > Arnaldo, how do we coordinate these patches? Should they go through > > bpf-next (after you Ack them) or you want them in your tree? > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so > > that I can do libbpf v0.8 release, having it in a separate tree is > > extremely inconvenient. Please let me know how you think we should > > proceed? > > we need to wait with perf changes after the libbpf is merged and > libbpf 0.8.0 is released.. so we don't break dynamic linking for > perf > > at the moment please just take libbpf change and I'll resend the > perf change later if needed Ok. - Arnaldo
On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu: > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote: > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > hi, > > > > sending change we discussed some time ago [1] to get rid of > > > > some deprecated functions we use in perf prologue code. > > > > > > > > Despite the gloomy discussion I think the final code does > > > > not look that bad ;-) > > > > > > > > This patchset removes following libbpf functions from perf: > > > > bpf_program__set_prep > > > > bpf_program__nth_fd > > > > struct bpf_prog_prep_result > > > > > > > > v2 changes: > > > > - use fallback section prog handler, so we don't need to > > > > use section prefix [Andrii] > > > > - realloc prog->insns array in bpf_program__set_insns [Andrii] > > > > - squash patch 1 from previous version with > > > > bpf_program__set_insns change [Daniel] > > > > - patch 3 already merged [Arnaldo] > > > > - added more comments > > > > > > > > meanwhile.. perf/core and bpf-next diverged, so: > > > > - libbpf bpf_program__set_insns change is based on bpf-next/master > > > > - perf changes do not apply on bpf-next/master so they are based on > > > > perf/core ... however they can be merged only after we release > > > > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > > > > the dynamic linking > > > > I'm sending perf changes now just for review, I'll resend them > > > > once libbpf 0.8.0 is released > > > > > > > > thanks, > > > > jirka > > > > > > > > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > > > > --- > > > > Jiri Olsa (1): > > > > libbpf: Add bpf_program__set_insns function > > > > > > > > > > The first patch looks good to me. The rest I can't really review and > > > test properly, so I'll leave it up to Arnaldo. > > > > > > Arnaldo, how do we coordinate these patches? Should they go through > > > bpf-next (after you Ack them) or you want them in your tree? > > > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so > > > that I can do libbpf v0.8 release, having it in a separate tree is > > > extremely inconvenient. Please let me know how you think we should > > > proceed? > > > > we need to wait with perf changes after the libbpf is merged and > > libbpf 0.8.0 is released.. so we don't break dynamic linking for > > perf > > > > at the moment please just take libbpf change and I'll resend the > > perf change later if needed > > Ok. > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > - Arnaldo
On Tue, May 17, 2022 at 3:03 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu: > > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote: > > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > > > hi, > > > > > sending change we discussed some time ago [1] to get rid of > > > > > some deprecated functions we use in perf prologue code. > > > > > > > > > > Despite the gloomy discussion I think the final code does > > > > > not look that bad ;-) > > > > > > > > > > This patchset removes following libbpf functions from perf: > > > > > bpf_program__set_prep > > > > > bpf_program__nth_fd > > > > > struct bpf_prog_prep_result > > > > > > > > > > v2 changes: > > > > > - use fallback section prog handler, so we don't need to > > > > > use section prefix [Andrii] > > > > > - realloc prog->insns array in bpf_program__set_insns [Andrii] > > > > > - squash patch 1 from previous version with > > > > > bpf_program__set_insns change [Daniel] > > > > > - patch 3 already merged [Arnaldo] > > > > > - added more comments > > > > > > > > > > meanwhile.. perf/core and bpf-next diverged, so: > > > > > - libbpf bpf_program__set_insns change is based on bpf-next/master > > > > > - perf changes do not apply on bpf-next/master so they are based on > > > > > perf/core ... however they can be merged only after we release > > > > > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > > > > > the dynamic linking > > > > > I'm sending perf changes now just for review, I'll resend them > > > > > once libbpf 0.8.0 is released > > > > > > > > > > thanks, > > > > > jirka > > > > > > > > > > > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > > > > > --- > > > > > Jiri Olsa (1): > > > > > libbpf: Add bpf_program__set_insns function > > > > > > > > > > > > > The first patch looks good to me. The rest I can't really review and > > > > test properly, so I'll leave it up to Arnaldo. > > > > > > > > Arnaldo, how do we coordinate these patches? Should they go through > > > > bpf-next (after you Ack them) or you want them in your tree? > > > > > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so > > > > that I can do libbpf v0.8 release, having it in a separate tree is > > > > extremely inconvenient. Please let me know how you think we should > > > > proceed? > > > > > > we need to wait with perf changes after the libbpf is merged and > > > libbpf 0.8.0 is released.. so we don't break dynamic linking for > > > perf > > > > > > at the moment please just take libbpf change and I'll resend the > > > perf change later if needed > > > > Ok. > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? We still haven't done as Andrii suggested in: https://lore.kernel.org/lkml/CAEf4BzbbOHQZUAe6iWaehKCPQAf3VC=hq657buqe2_yRKxaK-A@mail.gmail.com/ so for LIBBPF_DYNAMIC I believe we're compiling against the header files in tools/lib rather than the installed libbpf's header files. This may have some unexpected consequences. Thanks, Ian > > > - Arnaldo
On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote: > On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu: > > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote: > > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > > > hi, > > > > > sending change we discussed some time ago [1] to get rid of > > > > > some deprecated functions we use in perf prologue code. > > > > > > > > > > Despite the gloomy discussion I think the final code does > > > > > not look that bad ;-) > > > > > > > > > > This patchset removes following libbpf functions from perf: > > > > > bpf_program__set_prep > > > > > bpf_program__nth_fd > > > > > struct bpf_prog_prep_result > > > > > > > > > > v2 changes: > > > > > - use fallback section prog handler, so we don't need to > > > > > use section prefix [Andrii] > > > > > - realloc prog->insns array in bpf_program__set_insns [Andrii] > > > > > - squash patch 1 from previous version with > > > > > bpf_program__set_insns change [Daniel] > > > > > - patch 3 already merged [Arnaldo] > > > > > - added more comments > > > > > > > > > > meanwhile.. perf/core and bpf-next diverged, so: > > > > > - libbpf bpf_program__set_insns change is based on bpf-next/master > > > > > - perf changes do not apply on bpf-next/master so they are based on > > > > > perf/core ... however they can be merged only after we release > > > > > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > > > > > the dynamic linking > > > > > I'm sending perf changes now just for review, I'll resend them > > > > > once libbpf 0.8.0 is released > > > > > > > > > > thanks, > > > > > jirka > > > > > > > > > > > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > > > > > --- > > > > > Jiri Olsa (1): > > > > > libbpf: Add bpf_program__set_insns function > > > > > > > > > > > > > The first patch looks good to me. The rest I can't really review and > > > > test properly, so I'll leave it up to Arnaldo. > > > > > > > > Arnaldo, how do we coordinate these patches? Should they go through > > > > bpf-next (after you Ack them) or you want them in your tree? > > > > > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so > > > > that I can do libbpf v0.8 release, having it in a separate tree is > > > > extremely inconvenient. Please let me know how you think we should > > > > proceed? > > > > > > we need to wait with perf changes after the libbpf is merged and > > > libbpf 0.8.0 is released.. so we don't break dynamic linking for > > > perf > > > > > > at the moment please just take libbpf change and I'll resend the > > > perf change later if needed > > > > Ok. > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? yep, just made new fedora package.. will resend the perf changes soon jirka
On Tue, May 17, 2022 at 09:45:56PM -0700, Ian Rogers wrote: > On Tue, May 17, 2022 at 3:03 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo > > <acme@kernel.org> wrote: > > > > > > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu: > > > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote: > > > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > > > > > hi, > > > > > > sending change we discussed some time ago [1] to get rid of > > > > > > some deprecated functions we use in perf prologue code. > > > > > > > > > > > > Despite the gloomy discussion I think the final code does > > > > > > not look that bad ;-) > > > > > > > > > > > > This patchset removes following libbpf functions from perf: > > > > > > bpf_program__set_prep > > > > > > bpf_program__nth_fd > > > > > > struct bpf_prog_prep_result > > > > > > > > > > > > v2 changes: > > > > > > - use fallback section prog handler, so we don't need to > > > > > > use section prefix [Andrii] > > > > > > - realloc prog->insns array in bpf_program__set_insns [Andrii] > > > > > > - squash patch 1 from previous version with > > > > > > bpf_program__set_insns change [Daniel] > > > > > > - patch 3 already merged [Arnaldo] > > > > > > - added more comments > > > > > > > > > > > > meanwhile.. perf/core and bpf-next diverged, so: > > > > > > - libbpf bpf_program__set_insns change is based on bpf-next/master > > > > > > - perf changes do not apply on bpf-next/master so they are based on > > > > > > perf/core ... however they can be merged only after we release > > > > > > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > > > > > > the dynamic linking > > > > > > I'm sending perf changes now just for review, I'll resend them > > > > > > once libbpf 0.8.0 is released > > > > > > > > > > > > thanks, > > > > > > jirka > > > > > > > > > > > > > > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > > > > > > --- > > > > > > Jiri Olsa (1): > > > > > > libbpf: Add bpf_program__set_insns function > > > > > > > > > > > > > > > > The first patch looks good to me. The rest I can't really review and > > > > > test properly, so I'll leave it up to Arnaldo. > > > > > > > > > > Arnaldo, how do we coordinate these patches? Should they go through > > > > > bpf-next (after you Ack them) or you want them in your tree? > > > > > > > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so > > > > > that I can do libbpf v0.8 release, having it in a separate tree is > > > > > extremely inconvenient. Please let me know how you think we should > > > > > proceed? > > > > > > > > we need to wait with perf changes after the libbpf is merged and > > > > libbpf 0.8.0 is released.. so we don't break dynamic linking for > > > > perf > > > > > > > > at the moment please just take libbpf change and I'll resend the > > > > perf change later if needed > > > > > > Ok. > > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > > We still haven't done as Andrii suggested in: > https://lore.kernel.org/lkml/CAEf4BzbbOHQZUAe6iWaehKCPQAf3VC=hq657buqe2_yRKxaK-A@mail.gmail.com/ > so for LIBBPF_DYNAMIC I believe we're compiling against the header > files in tools/lib rather than the installed libbpf's header files. > This may have some unexpected consequences. like your program is not doing what you expected.. been there ;-) sounds good, will check jirka
On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote: > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote: > > On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo > > <acme@kernel.org> wrote: > > > > > > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu: > > > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote: > > > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > > > > > hi, > > > > > > sending change we discussed some time ago [1] to get rid of > > > > > > some deprecated functions we use in perf prologue code. > > > > > > > > > > > > Despite the gloomy discussion I think the final code does > > > > > > not look that bad ;-) > > > > > > > > > > > > This patchset removes following libbpf functions from perf: > > > > > > bpf_program__set_prep > > > > > > bpf_program__nth_fd > > > > > > struct bpf_prog_prep_result > > > > > > > > > > > > v2 changes: > > > > > > - use fallback section prog handler, so we don't need to > > > > > > use section prefix [Andrii] > > > > > > - realloc prog->insns array in bpf_program__set_insns [Andrii] > > > > > > - squash patch 1 from previous version with > > > > > > bpf_program__set_insns change [Daniel] > > > > > > - patch 3 already merged [Arnaldo] > > > > > > - added more comments > > > > > > > > > > > > meanwhile.. perf/core and bpf-next diverged, so: > > > > > > - libbpf bpf_program__set_insns change is based on bpf-next/master > > > > > > - perf changes do not apply on bpf-next/master so they are based on > > > > > > perf/core ... however they can be merged only after we release > > > > > > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > > > > > > the dynamic linking > > > > > > I'm sending perf changes now just for review, I'll resend them > > > > > > once libbpf 0.8.0 is released > > > > > > > > > > > > thanks, > > > > > > jirka > > > > > > > > > > > > > > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > > > > > > --- > > > > > > Jiri Olsa (1): > > > > > > libbpf: Add bpf_program__set_insns function > > > > > > > > > > > > > > > > The first patch looks good to me. The rest I can't really review and > > > > > test properly, so I'll leave it up to Arnaldo. > > > > > > > > > > Arnaldo, how do we coordinate these patches? Should they go through > > > > > bpf-next (after you Ack them) or you want them in your tree? > > > > > > > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so > > > > > that I can do libbpf v0.8 release, having it in a separate tree is > > > > > extremely inconvenient. Please let me know how you think we should > > > > > proceed? > > > > > > > > we need to wait with perf changes after the libbpf is merged and > > > > libbpf 0.8.0 is released.. so we don't break dynamic linking for > > > > perf > > > > > > > > at the moment please just take libbpf change and I'll resend the > > > > perf change later if needed > > > > > > Ok. > > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > > yep, just made new fedora package.. will resend the perf changes soon fedora package is on the way, but I'll need perf/core to merge the bpf_program__set_insns change.. Arnaldo, any idea when this could happen? thanks, jirka
On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote: > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote: > > > On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo > > > <acme@kernel.org> wrote: > > > > > > > > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu: > > > > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote: > > > > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > > > > > > > hi, > > > > > > > sending change we discussed some time ago [1] to get rid of > > > > > > > some deprecated functions we use in perf prologue code. > > > > > > > > > > > > > > Despite the gloomy discussion I think the final code does > > > > > > > not look that bad ;-) > > > > > > > > > > > > > > This patchset removes following libbpf functions from perf: > > > > > > > bpf_program__set_prep > > > > > > > bpf_program__nth_fd > > > > > > > struct bpf_prog_prep_result > > > > > > > > > > > > > > v2 changes: > > > > > > > - use fallback section prog handler, so we don't need to > > > > > > > use section prefix [Andrii] > > > > > > > - realloc prog->insns array in bpf_program__set_insns [Andrii] > > > > > > > - squash patch 1 from previous version with > > > > > > > bpf_program__set_insns change [Daniel] > > > > > > > - patch 3 already merged [Arnaldo] > > > > > > > - added more comments > > > > > > > > > > > > > > meanwhile.. perf/core and bpf-next diverged, so: > > > > > > > - libbpf bpf_program__set_insns change is based on bpf-next/master > > > > > > > - perf changes do not apply on bpf-next/master so they are based on > > > > > > > perf/core ... however they can be merged only after we release > > > > > > > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > > > > > > > the dynamic linking > > > > > > > I'm sending perf changes now just for review, I'll resend them > > > > > > > once libbpf 0.8.0 is released > > > > > > > > > > > > > > thanks, > > > > > > > jirka > > > > > > > > > > > > > > > > > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > > > > > > > --- > > > > > > > Jiri Olsa (1): > > > > > > > libbpf: Add bpf_program__set_insns function > > > > > > > > > > > > > > > > > > > The first patch looks good to me. The rest I can't really review and > > > > > > test properly, so I'll leave it up to Arnaldo. > > > > > > > > > > > > Arnaldo, how do we coordinate these patches? Should they go through > > > > > > bpf-next (after you Ack them) or you want them in your tree? > > > > > > > > > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so > > > > > > that I can do libbpf v0.8 release, having it in a separate tree is > > > > > > extremely inconvenient. Please let me know how you think we should > > > > > > proceed? > > > > > > > > > > we need to wait with perf changes after the libbpf is merged and > > > > > libbpf 0.8.0 is released.. so we don't break dynamic linking for > > > > > perf > > > > > > > > > > at the moment please just take libbpf change and I'll resend the > > > > > perf change later if needed > > > > > > > > Ok. > > > > > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > > > > yep, just made new fedora package.. will resend the perf changes soon > > fedora package is on the way, but I'll need perf/core to merge > the bpf_program__set_insns change.. Arnaldo, any idea when this > could happen? > Jiri, Arnaldo, Can we land these patches through bpf-next to avoid such complicated cross-tree dependencies? As I started removing libbpf APIs I also noticed that perf is still using few other deprecated APIs: - bpf_map__next; - bpf_program__next; - bpf_load_program; - btf__get_from_id; It's trivial to fix up, but doing it across few trees will delay libbpf work as well. So let's land this through bpf-next, if Arnaldo doesn't mind? > thanks, > jirka
Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu: > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote: > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote: > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > > > yep, just made new fedora package.. will resend the perf changes soon > > fedora package is on the way, but I'll need perf/core to merge > > the bpf_program__set_insns change.. Arnaldo, any idea when this > > could happen? > Can we land these patches through bpf-next to avoid such complicated > cross-tree dependencies? As I started removing libbpf APIs I also > noticed that perf is still using few other deprecated APIs: > - bpf_map__next; > - bpf_program__next; > - bpf_load_program; > - btf__get_from_id; > It's trivial to fix up, but doing it across few trees will delay > libbpf work as well. > So let's land this through bpf-next, if Arnaldo doesn't mind? Yeah, that should be ok, the only consideration is that I'm submitting this today to Linus: https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351 To address this: https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/ - Arnaldo
On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote: > Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu: > > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote: > > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote: > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > > > > > yep, just made new fedora package.. will resend the perf changes soon > > > > fedora package is on the way, but I'll need perf/core to merge > > > the bpf_program__set_insns change.. Arnaldo, any idea when this > > > could happen? > > > Can we land these patches through bpf-next to avoid such complicated > > cross-tree dependencies? As I started removing libbpf APIs I also > > noticed that perf is still using few other deprecated APIs: > > - bpf_map__next; > > - bpf_program__next; > > - bpf_load_program; > > - btf__get_from_id; these were added just to bypass the time window when they were not available in the package, so can be removed now (in the patch below) > > > It's trivial to fix up, but doing it across few trees will delay > > libbpf work as well. > > > So let's land this through bpf-next, if Arnaldo doesn't mind? > > Yeah, that should be ok, the only consideration is that I'm submitting > this today to Linus: > > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351 > > To address this: > > https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/ ok, we can do that via bpf-next, but of course there's a problem ;-) perf/core already has dependency commit [1] so either we wait for perf/core and bpf-next/master to sync or: - perf/core reverts [1] and - bpf-next/master takes [1] and the rest I have the changes ready if you guys are ok with that thanks, jirka [1] https://lore.kernel.org/bpf/20220422100025.1469207-4-jolsa@kernel.org/ --- Subject: [PATCH bpf-next] perf tools: Remove the weak libbpf functions We added weak functions for some new libbpf functions because they were not packaged at that time [1]. These functions are now available in package, so we can remove their weak perf variants. [1] https://lore.kernel.org/linux-perf-users/20211109140707.1689940-2-jolsa@kernel.org/ Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/perf/util/bpf-event.c | 51 ------------------------------------- 1 file changed, 51 deletions(-) diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c index 94624733af7e..025f331b3867 100644 --- a/tools/perf/util/bpf-event.c +++ b/tools/perf/util/bpf-event.c @@ -22,57 +22,6 @@ #include "record.h" #include "util/synthetic-events.h" -struct btf * __weak btf__load_from_kernel_by_id(__u32 id) -{ - struct btf *btf; -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" - int err = btf__get_from_id(id, &btf); -#pragma GCC diagnostic pop - - return err ? ERR_PTR(err) : btf; -} - -int __weak bpf_prog_load(enum bpf_prog_type prog_type, - const char *prog_name __maybe_unused, - const char *license, - const struct bpf_insn *insns, size_t insn_cnt, - const struct bpf_prog_load_opts *opts) -{ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" - return bpf_load_program(prog_type, insns, insn_cnt, license, - opts->kern_version, opts->log_buf, opts->log_size); -#pragma GCC diagnostic pop -} - -struct bpf_program * __weak -bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev) -{ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" - return bpf_program__next(prev, obj); -#pragma GCC diagnostic pop -} - -struct bpf_map * __weak -bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev) -{ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" - return bpf_map__next(prev, obj); -#pragma GCC diagnostic pop -} - -const void * __weak -btf__raw_data(const struct btf *btf_ro, __u32 *size) -{ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" - return btf__get_raw_data(btf_ro, size); -#pragma GCC diagnostic pop -} - static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len) { int ret = 0;
On Mon, May 23, 2022 at 12:49 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote: > > Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu: > > > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote: > > > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote: > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > > > > > > > yep, just made new fedora package.. will resend the perf changes soon > > > > > > fedora package is on the way, but I'll need perf/core to merge > > > > the bpf_program__set_insns change.. Arnaldo, any idea when this > > > > could happen? > > > > > Can we land these patches through bpf-next to avoid such complicated > > > cross-tree dependencies? As I started removing libbpf APIs I also > > > noticed that perf is still using few other deprecated APIs: > > > - bpf_map__next; > > > - bpf_program__next; > > > - bpf_load_program; > > > - btf__get_from_id; > > these were added just to bypass the time window when they were not > available in the package, so can be removed now (in the patch below) > > > > > > It's trivial to fix up, but doing it across few trees will delay > > > libbpf work as well. > > > > > So let's land this through bpf-next, if Arnaldo doesn't mind? > > > > Yeah, that should be ok, the only consideration is that I'm submitting > > this today to Linus: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351 > > > > To address this: > > > > https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/ > > ok, we can do that via bpf-next, but of course there's a problem ;-) > > perf/core already has dependency commit [1] > > so either we wait for perf/core and bpf-next/master to sync or: > > - perf/core reverts [1] and > - bpf-next/master takes [1] and the rest > > I have the changes ready if you guys are ok with that So, if I understand correctly, with merge window open bpf-next/master will get code from perf/core soon when we merge tip back in. So we can wait for that to happen and not revert anything. So please add the below patch to your series and resend once tip is merged into bpf-next? Thanks! > > thanks, > jirka > > > [1] https://lore.kernel.org/bpf/20220422100025.1469207-4-jolsa@kernel.org/ > > --- > Subject: [PATCH bpf-next] perf tools: Remove the weak libbpf functions > > We added weak functions for some new libbpf functions because > they were not packaged at that time [1]. > > These functions are now available in package, so we can remove > their weak perf variants. > > [1] https://lore.kernel.org/linux-perf-users/20211109140707.1689940-2-jolsa@kernel.org/ > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/perf/util/bpf-event.c | 51 ------------------------------------- > 1 file changed, 51 deletions(-) > > diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c > index 94624733af7e..025f331b3867 100644 > --- a/tools/perf/util/bpf-event.c > +++ b/tools/perf/util/bpf-event.c > @@ -22,57 +22,6 @@ > #include "record.h" > #include "util/synthetic-events.h" > > -struct btf * __weak btf__load_from_kernel_by_id(__u32 id) > -{ > - struct btf *btf; > -#pragma GCC diagnostic push > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" > - int err = btf__get_from_id(id, &btf); > -#pragma GCC diagnostic pop > - > - return err ? ERR_PTR(err) : btf; > -} > - > -int __weak bpf_prog_load(enum bpf_prog_type prog_type, > - const char *prog_name __maybe_unused, > - const char *license, > - const struct bpf_insn *insns, size_t insn_cnt, > - const struct bpf_prog_load_opts *opts) > -{ > -#pragma GCC diagnostic push > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" > - return bpf_load_program(prog_type, insns, insn_cnt, license, > - opts->kern_version, opts->log_buf, opts->log_size); > -#pragma GCC diagnostic pop > -} > - > -struct bpf_program * __weak > -bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev) > -{ > -#pragma GCC diagnostic push > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" > - return bpf_program__next(prev, obj); > -#pragma GCC diagnostic pop > -} > - > -struct bpf_map * __weak > -bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev) > -{ > -#pragma GCC diagnostic push > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" > - return bpf_map__next(prev, obj); > -#pragma GCC diagnostic pop > -} > - > -const void * __weak > -btf__raw_data(const struct btf *btf_ro, __u32 *size) > -{ > -#pragma GCC diagnostic push > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" > - return btf__get_raw_data(btf_ro, size); > -#pragma GCC diagnostic pop > -} > - > static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len) > { > int ret = 0; > -- > 2.35.3 >
On Mon, May 23, 2022 at 03:43:10PM -0700, Andrii Nakryiko wrote: > On Mon, May 23, 2022 at 12:49 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote: > > > Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu: > > > > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote: > > > > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote: > > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > > > > > > > > > yep, just made new fedora package.. will resend the perf changes soon > > > > > > > > fedora package is on the way, but I'll need perf/core to merge > > > > > the bpf_program__set_insns change.. Arnaldo, any idea when this > > > > > could happen? > > > > > > > Can we land these patches through bpf-next to avoid such complicated > > > > cross-tree dependencies? As I started removing libbpf APIs I also > > > > noticed that perf is still using few other deprecated APIs: > > > > - bpf_map__next; > > > > - bpf_program__next; > > > > - bpf_load_program; > > > > - btf__get_from_id; > > > > these were added just to bypass the time window when they were not > > available in the package, so can be removed now (in the patch below) > > > > > > > > > It's trivial to fix up, but doing it across few trees will delay > > > > libbpf work as well. > > > > > > > So let's land this through bpf-next, if Arnaldo doesn't mind? > > > > > > Yeah, that should be ok, the only consideration is that I'm submitting > > > this today to Linus: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351 > > > > > > To address this: > > > > > > https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/ > > > > ok, we can do that via bpf-next, but of course there's a problem ;-) > > > > perf/core already has dependency commit [1] > > > > so either we wait for perf/core and bpf-next/master to sync or: > > > > - perf/core reverts [1] and > > - bpf-next/master takes [1] and the rest > > > > I have the changes ready if you guys are ok with that > > So, if I understand correctly, with merge window open bpf-next/master > will get code from perf/core soon when we merge tip back in. So we can > wait for that to happen and not revert anything. > > So please add the below patch to your series and resend once tip is > merged into bpf-next? Thanks! ok jirka
Em Tue, May 10, 2022 at 09:46:56AM +0200, Jiri Olsa escreveu: > hi, > sending change we discussed some time ago [1] to get rid of > some deprecated functions we use in perf prologue code. > > Despite the gloomy discussion I think the final code does > not look that bad ;-) > > This patchset removes following libbpf functions from perf: > bpf_program__set_prep > bpf_program__nth_fd > struct bpf_prog_prep_result So, the first patch is already in torvalds/master, I tried applying the other two patches to my local perf/core, that already is merged with torvalds/master and: [root@quaco ~]# perf test 42 42: BPF filter : 42.1: Basic BPF filtering : FAILED! 42.2: BPF pinning : FAILED! 42.3: BPF prologue generation : FAILED! [root@quaco ~]# I'll push my local perf/core to tmp.perf/core and continue tomorrow. Its failing around here: Open Debuginfo file: /root/.cache/debuginfod_client/e1c3de4b4c5db158f2098e80f2bf9140e8cfbdb6/debuginfo Try to find probe point from debuginfo. Matched function: do_epoll_wait [3806bb5] Probe point found: do_epoll_wait+0 Found 1 probe_trace_events. Looking at the vmlinux_path (8 entries long) symsrc__init: build id mismatch for vmlinux. symsrc__init: cannot get elf header. Using /proc/kcore for kernel data Using /proc/kallsyms for symbols do_epoll_wait is out of .text, skip it. Post processing failed or all events are skipped. (1) Probe point 'do_epoll_wait' not found. bpf_probe: failed to convert perf probe events Failed to add events selected by BPF test child finished with -1 ---- end ---- BPF filter subtest 1: FAILED But: [root@quaco ~]# grep do_epoll_wait /proc/kallsyms ffffffff973c2a30 t do_epoll_wait [root@quaco ~]# - Arnaldo > v2 changes: > - use fallback section prog handler, so we don't need to > use section prefix [Andrii] > - realloc prog->insns array in bpf_program__set_insns [Andrii] > - squash patch 1 from previous version with > bpf_program__set_insns change [Daniel] > - patch 3 already merged [Arnaldo] > - added more comments > > meanwhile.. perf/core and bpf-next diverged, so: > - libbpf bpf_program__set_insns change is based on bpf-next/master > - perf changes do not apply on bpf-next/master so they are based on > perf/core ... however they can be merged only after we release > libbpf 0.8.0 with bpf_program__set_insns change, so we don't break > the dynamic linking > I'm sending perf changes now just for review, I'll resend them > once libbpf 0.8.0 is released > > thanks, > jirka > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ > --- > Jiri Olsa (1): > libbpf: Add bpf_program__set_insns function > > tools/lib/bpf/libbpf.c | 22 ++++++++++++++++++++++ > tools/lib/bpf/libbpf.h | 18 ++++++++++++++++++ > tools/lib/bpf/libbpf.map | 1 + > 3 files changed, 41 insertions(+) > > Jiri Olsa (2): > perf tools: Register fallback libbpf section handler > perf tools: Rework prologue generation code > > tools/perf/util/bpf-loader.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 157 insertions(+), 18 deletions(-)
On Tue, May 24, 2022 at 1:28 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Mon, May 23, 2022 at 03:43:10PM -0700, Andrii Nakryiko wrote: > > On Mon, May 23, 2022 at 12:49 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote: > > > > Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu: > > > > > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote: > > > > > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote: > > > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > > > > > > > > > > > yep, just made new fedora package.. will resend the perf changes soon > > > > > > > > > > fedora package is on the way, but I'll need perf/core to merge > > > > > > the bpf_program__set_insns change.. Arnaldo, any idea when this > > > > > > could happen? > > > > > > > > > Can we land these patches through bpf-next to avoid such complicated > > > > > cross-tree dependencies? As I started removing libbpf APIs I also > > > > > noticed that perf is still using few other deprecated APIs: > > > > > - bpf_map__next; > > > > > - bpf_program__next; > > > > > - bpf_load_program; > > > > > - btf__get_from_id; > > > > > > these were added just to bypass the time window when they were not > > > available in the package, so can be removed now (in the patch below) > > > > > > > > > > > > It's trivial to fix up, but doing it across few trees will delay > > > > > libbpf work as well. > > > > > > > > > So let's land this through bpf-next, if Arnaldo doesn't mind? > > > > > > > > Yeah, that should be ok, the only consideration is that I'm submitting > > > > this today to Linus: > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351 > > > > > > > > To address this: > > > > > > > > https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/ > > > > > > ok, we can do that via bpf-next, but of course there's a problem ;-) > > > > > > perf/core already has dependency commit [1] > > > > > > so either we wait for perf/core and bpf-next/master to sync or: > > > > > > - perf/core reverts [1] and > > > - bpf-next/master takes [1] and the rest > > > > > > I have the changes ready if you guys are ok with that > > > > So, if I understand correctly, with merge window open bpf-next/master > > will get code from perf/core soon when we merge tip back in. So we can > > wait for that to happen and not revert anything. > > > > So please add the below patch to your series and resend once tip is > > merged into bpf-next? Thanks! > > ok Hm.. Ok, so I don't see your patches in tip yet. I see them in perf/core only. Which means things won't happen naturally soon. How should we proceed? I'm sitting on a pile of patches removing a lot of code from libbpf and I'd rather get it out soon, but I can't because of them breaking perf in bpf-next without Jiri's changes. Arnaldo, what's your suggestion? Can we land remaining Jiri's patches into perf/core and then you can create a tag for us to merge into bpf-next, so that we avoid any conflicts later? Would that work? I think we did something like that with other trees (e.g., RCU), when we had dependencies like this before. Thoughts? > > jirka
On Wed, Jun 01, 2022 at 10:39:08AM -0700, Andrii Nakryiko wrote: > On Tue, May 24, 2022 at 1:28 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > On Mon, May 23, 2022 at 03:43:10PM -0700, Andrii Nakryiko wrote: > > > On Mon, May 23, 2022 at 12:49 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > > On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote: > > > > > Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu: > > > > > > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote: > > > > > > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote: > > > > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches? > > > > > > > > > > > > > yep, just made new fedora package.. will resend the perf changes soon > > > > > > > > > > > > fedora package is on the way, but I'll need perf/core to merge > > > > > > > the bpf_program__set_insns change.. Arnaldo, any idea when this > > > > > > > could happen? > > > > > > > > > > > Can we land these patches through bpf-next to avoid such complicated > > > > > > cross-tree dependencies? As I started removing libbpf APIs I also > > > > > > noticed that perf is still using few other deprecated APIs: > > > > > > - bpf_map__next; > > > > > > - bpf_program__next; > > > > > > - bpf_load_program; > > > > > > - btf__get_from_id; > > > > > > > > these were added just to bypass the time window when they were not > > > > available in the package, so can be removed now (in the patch below) > > > > > > > > > > > > > > > It's trivial to fix up, but doing it across few trees will delay > > > > > > libbpf work as well. > > > > > > > > > > > So let's land this through bpf-next, if Arnaldo doesn't mind? > > > > > > > > > > Yeah, that should be ok, the only consideration is that I'm submitting > > > > > this today to Linus: > > > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351 > > > > > > > > > > To address this: > > > > > > > > > > https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/ > > > > > > > > ok, we can do that via bpf-next, but of course there's a problem ;-) > > > > > > > > perf/core already has dependency commit [1] > > > > > > > > so either we wait for perf/core and bpf-next/master to sync or: > > > > > > > > - perf/core reverts [1] and > > > > - bpf-next/master takes [1] and the rest > > > > > > > > I have the changes ready if you guys are ok with that > > > > > > So, if I understand correctly, with merge window open bpf-next/master > > > will get code from perf/core soon when we merge tip back in. So we can > > > wait for that to happen and not revert anything. > > > > > > So please add the below patch to your series and resend once tip is > > > merged into bpf-next? Thanks! > > > > ok > > Hm.. Ok, so I don't see your patches in tip yet. I see them in > perf/core only. Which means things won't happen naturally soon. How > should we proceed? I'm sitting on a pile of patches removing a lot of > code from libbpf and I'd rather get it out soon, but I can't because > of them breaking perf in bpf-next without Jiri's changes. sorry it's merged in linus master, Arnaldo no longer goes through tip tree > > Arnaldo, what's your suggestion? Can we land remaining Jiri's patches > into perf/core and then you can create a tag for us to merge into > bpf-next, so that we avoid any conflicts later? Would that work? I > think we did something like that with other trees (e.g., RCU), when we > had dependencies like this before. either way is fine for me.. I just rebased those changes on top of perf/core jirka
On Thu, May 26, 2022 at 10:16:11PM -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, May 10, 2022 at 09:46:56AM +0200, Jiri Olsa escreveu: > > hi, > > sending change we discussed some time ago [1] to get rid of > > some deprecated functions we use in perf prologue code. > > > > Despite the gloomy discussion I think the final code does > > not look that bad ;-) > > > > This patchset removes following libbpf functions from perf: > > bpf_program__set_prep > > bpf_program__nth_fd > > struct bpf_prog_prep_result > > So, the first patch is already in torvalds/master, I tried applying the > other two patches to my local perf/core, that already is merged with > torvalds/master and: > > [root@quaco ~]# perf test 42 > 42: BPF filter : > 42.1: Basic BPF filtering : FAILED! > 42.2: BPF pinning : FAILED! > 42.3: BPF prologue generation : FAILED! > [root@quaco ~]# > > I'll push my local perf/core to tmp.perf/core and continue tomorrow. hi, I just rebased my changes on top of your perf/core and it seems to work: [root@krava perf]# ./perf test bpf 40: LLVM search and compile : 40.1: Basic BPF llvm compile : Ok 40.3: Compile source for BPF prologue generation : Ok 40.4: Compile source for BPF relocation : Ok 42: BPF filter : 42.1: Basic BPF filtering : Ok 42.2: BPF pinning : Ok 42.3: BPF prologue generation : Ok is it still a problem? jirka
On Wed, Jun 1, 2022 at 11:11 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Thu, May 26, 2022 at 10:16:11PM -0300, Arnaldo Carvalho de Melo wrote: > > Em Tue, May 10, 2022 at 09:46:56AM +0200, Jiri Olsa escreveu: > > > hi, > > > sending change we discussed some time ago [1] to get rid of > > > some deprecated functions we use in perf prologue code. > > > > > > Despite the gloomy discussion I think the final code does > > > not look that bad ;-) > > > > > > This patchset removes following libbpf functions from perf: > > > bpf_program__set_prep > > > bpf_program__nth_fd > > > struct bpf_prog_prep_result > > > > So, the first patch is already in torvalds/master, I tried applying the > > other two patches to my local perf/core, that already is merged with > > torvalds/master and: > > > > [root@quaco ~]# perf test 42 > > 42: BPF filter : > > 42.1: Basic BPF filtering : FAILED! > > 42.2: BPF pinning : FAILED! > > 42.3: BPF prologue generation : FAILED! > > [root@quaco ~]# > > > > I'll push my local perf/core to tmp.perf/core and continue tomorrow. > > hi, > I just rebased my changes on top of your perf/core and it seems to work: > > [root@krava perf]# ./perf test bpf > 40: LLVM search and compile : > 40.1: Basic BPF llvm compile : Ok > 40.3: Compile source for BPF prologue generation : Ok > 40.4: Compile source for BPF relocation : Ok > 42: BPF filter : > 42.1: Basic BPF filtering : Ok > 42.2: BPF pinning : Ok > 42.3: BPF prologue generation : Ok > > is it still a problem? > Ok, so I checked with Jakub, net-next will be forwarded to linus/master tomorrow or so, so after that bpf-next will get forwarded as well and we'll have all those patches of yours. So let's go back to plan A: send your perf changes based on bpf-next. Thanks and sorry for the extra noise with all the back and forth. > jirka
On Wed, Jun 01, 2022 at 03:21:06PM -0700, Andrii Nakryiko wrote: > On Wed, Jun 1, 2022 at 11:11 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > On Thu, May 26, 2022 at 10:16:11PM -0300, Arnaldo Carvalho de Melo wrote: > > > Em Tue, May 10, 2022 at 09:46:56AM +0200, Jiri Olsa escreveu: > > > > hi, > > > > sending change we discussed some time ago [1] to get rid of > > > > some deprecated functions we use in perf prologue code. > > > > > > > > Despite the gloomy discussion I think the final code does > > > > not look that bad ;-) > > > > > > > > This patchset removes following libbpf functions from perf: > > > > bpf_program__set_prep > > > > bpf_program__nth_fd > > > > struct bpf_prog_prep_result > > > > > > So, the first patch is already in torvalds/master, I tried applying the > > > other two patches to my local perf/core, that already is merged with > > > torvalds/master and: > > > > > > [root@quaco ~]# perf test 42 > > > 42: BPF filter : > > > 42.1: Basic BPF filtering : FAILED! > > > 42.2: BPF pinning : FAILED! > > > 42.3: BPF prologue generation : FAILED! > > > [root@quaco ~]# > > > > > > I'll push my local perf/core to tmp.perf/core and continue tomorrow. > > > > hi, > > I just rebased my changes on top of your perf/core and it seems to work: > > > > [root@krava perf]# ./perf test bpf > > 40: LLVM search and compile : > > 40.1: Basic BPF llvm compile : Ok > > 40.3: Compile source for BPF prologue generation : Ok > > 40.4: Compile source for BPF relocation : Ok > > 42: BPF filter : > > 42.1: Basic BPF filtering : Ok > > 42.2: BPF pinning : Ok > > 42.3: BPF prologue generation : Ok > > > > is it still a problem? > > > > Ok, so I checked with Jakub, net-next will be forwarded to > linus/master tomorrow or so, so after that bpf-next will get forwarded > as well and we'll have all those patches of yours. So let's go back to > plan A: send your perf changes based on bpf-next. Thanks and sorry for > the extra noise with all the back and forth. ok, np jirka
hi, sending change we discussed some time ago [1] to get rid of some deprecated functions we use in perf prologue code. Despite the gloomy discussion I think the final code does not look that bad ;-) This patchset removes following libbpf functions from perf: bpf_program__set_prep bpf_program__nth_fd struct bpf_prog_prep_result v2 changes: - use fallback section prog handler, so we don't need to use section prefix [Andrii] - realloc prog->insns array in bpf_program__set_insns [Andrii] - squash patch 1 from previous version with bpf_program__set_insns change [Daniel] - patch 3 already merged [Arnaldo] - added more comments meanwhile.. perf/core and bpf-next diverged, so: - libbpf bpf_program__set_insns change is based on bpf-next/master - perf changes do not apply on bpf-next/master so they are based on perf/core ... however they can be merged only after we release libbpf 0.8.0 with bpf_program__set_insns change, so we don't break the dynamic linking I'm sending perf changes now just for review, I'll resend them once libbpf 0.8.0 is released thanks, jirka [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/ --- Jiri Olsa (1): libbpf: Add bpf_program__set_insns function tools/lib/bpf/libbpf.c | 22 ++++++++++++++++++++++ tools/lib/bpf/libbpf.h | 18 ++++++++++++++++++ tools/lib/bpf/libbpf.map | 1 + 3 files changed, 41 insertions(+) Jiri Olsa (2): perf tools: Register fallback libbpf section handler perf tools: Rework prologue generation code tools/perf/util/bpf-loader.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 157 insertions(+), 18 deletions(-)