Re: [BUG] perf test: Regression because of d6e6286a12e7

Re: [BUG] perf test: Regression because of d6e6286a12e7

[Andrii Nakryiko]

On Sun, Jul 9, 2023 at 9:05 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> On 08-Jul-23 4:46 AM, Andrii Nakryiko wrote:
> > On Wed, Jul 5, 2023 at 9:39 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
> >>
> >> Hi Andrii,
> >>
> >> I'm seeing perf test failure because of commit d6e6286a12e7 ("libbpf:
> >> disassociate section handler on explicit bpf_program__set_type() call").
> >>
> >
> > Yep, this commit would reset catch-all custom handler, which perf is
> > setting. I've just sent a fix upstream ([0]). And once it lands, I'll
> > cut a v1.2.1 libbpf bugfix release with just this fix on top of v1.2.
> >
> > Can you please double-check that this patch indeed fixes the issue for
> > you? I tried to do this locally, but for me perf test 42 fails both at
> > current bpf-next, with the above commit reverted, and with my fix
> > applied on top. So I can't be 100% sure.
> >
> > Thanks!
> >
> >   [0] https://patchwork.kernel.org/project/netdevbpf/patch/20230707231156.1711948-1-andrii@kernel.org/
>
> Thanks. A quick test seems to be working fine.

Alright, thanks for confirming! I've just released v1.2.1 bug fix
release with just this fix on top of v1.2.

Thanks for reporting!

But given v1.2 was cut on May 1st, and the offending commit landed
some time late March, I wonder how did this slip through the cracks
and go unreported for so long? Is there something we can do to catch
these perf-only regressions a bit sooner?


>
> Ravi

Re: [BUG] perf test: Regression because of d6e6286a12e7

[Ravi Bangoria]

On 11-Jul-23 3:06 AM, Andrii Nakryiko wrote:
> On Sun, Jul 9, 2023 at 9:05 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>>
>> On 08-Jul-23 4:46 AM, Andrii Nakryiko wrote:
>>> On Wed, Jul 5, 2023 at 9:39 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>>>>
>>>> Hi Andrii,
>>>>
>>>> I'm seeing perf test failure because of commit d6e6286a12e7 ("libbpf:
>>>> disassociate section handler on explicit bpf_program__set_type() call").
>>>>
>>>
>>> Yep, this commit would reset catch-all custom handler, which perf is
>>> setting. I've just sent a fix upstream ([0]). And once it lands, I'll
>>> cut a v1.2.1 libbpf bugfix release with just this fix on top of v1.2.
>>>
>>> Can you please double-check that this patch indeed fixes the issue for
>>> you? I tried to do this locally, but for me perf test 42 fails both at
>>> current bpf-next, with the above commit reverted, and with my fix
>>> applied on top. So I can't be 100% sure.
>>>
>>> Thanks!
>>>
>>>   [0] https://patchwork.kernel.org/project/netdevbpf/patch/20230707231156.1711948-1-andrii@kernel.org/
>>
>> Thanks. A quick test seems to be working fine.
> 
> Alright, thanks for confirming! I've just released v1.2.1 bug fix
> release with just this fix on top of v1.2.
> 
> Thanks for reporting!
> 
> But given v1.2 was cut on May 1st, and the offending commit landed
> some time late March, I wonder how did this slip through the cracks
> and go unreported for so long? Is there something we can do to catch
> these perf-only regressions a bit sooner?

I guess it got slipped because that patch went in via bpf tree. Would
it be possible to run bpf related perf tests at the time of applying
libbpf patches? Arnaldo might have better ideas :)

Thanks,
Ravi

Re: [BUG] perf test: Regression because of d6e6286a12e7

[Arnaldo Carvalho de Melo]

Em Wed, Jul 12, 2023 at 07:38:58PM +0530, Ravi Bangoria escreveu:
> On 11-Jul-23 3:06 AM, Andrii Nakryiko wrote:
> > On Sun, Jul 9, 2023 at 9:05 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
> >> On 08-Jul-23 4:46 AM, Andrii Nakryiko wrote:
> >>> On Wed, Jul 5, 2023 at 9:39 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
> >>>> I'm seeing perf test failure because of commit d6e6286a12e7 ("libbpf:
> >>>> disassociate section handler on explicit bpf_program__set_type() call").

> >>> Yep, this commit would reset catch-all custom handler, which perf is
> >>> setting. I've just sent a fix upstream ([0]). And once it lands, I'll
> >>> cut a v1.2.1 libbpf bugfix release with just this fix on top of v1.2.

> >>> Can you please double-check that this patch indeed fixes the issue for
> >>> you? I tried to do this locally, but for me perf test 42 fails both at
> >>> current bpf-next, with the above commit reverted, and with my fix
> >>> applied on top. So I can't be 100% sure.

> >>>   [0] https://patchwork.kernel.org/project/netdevbpf/patch/20230707231156.1711948-1-andrii@kernel.org/

> >> Thanks. A quick test seems to be working fine.

> > Alright, thanks for confirming! I've just released v1.2.1 bug fix
> > release with just this fix on top of v1.2.

> > Thanks for reporting!

> > But given v1.2 was cut on May 1st, and the offending commit landed
> > some time late March, I wonder how did this slip through the cracks
> > and go unreported for so long? Is there something we can do to catch
> > these perf-only regressions a bit sooner?

> I guess it got slipped because that patch went in via bpf tree. Would
> it be possible to run bpf related perf tests at the time of applying
> libbpf patches? Arnaldo might have better ideas :)

Right, perhaps the libbpf CI could try building perf, preferably with
BUILD_BPF_SKEL=1, to enable these tools:

[acme@nine linux]$ ls -la tools/perf/util/bpf_skel/*.bpf.*
-rw-r--r--. 1 acme acme  5581 Jul  7 12:38 tools/perf/util/bpf_skel/bperf_cgroup.bpf.c
-rw-r--r--. 1 acme acme  1764 Jul  7 12:38 tools/perf/util/bpf_skel/bperf_follower.bpf.c
-rw-r--r--. 1 acme acme  1438 Jul  7 12:38 tools/perf/util/bpf_skel/bperf_leader.bpf.c
-rw-r--r--. 1 acme acme  2290 Jul  7 12:38 tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
-rw-r--r--. 1 acme acme  2164 Jul  7 12:38 tools/perf/util/bpf_skel/func_latency.bpf.c
-rw-r--r--. 1 acme acme  9017 Jul  7 12:38 tools/perf/util/bpf_skel/kwork_trace.bpf.c
-rw-r--r--. 1 acme acme 10147 Jul 12 11:49 tools/perf/util/bpf_skel/lock_contention.bpf.c
-rw-r--r--. 1 acme acme  6109 Jul  7 12:38 tools/perf/util/bpf_skel/off_cpu.bpf.c
-rw-r--r--. 1 acme acme  4932 Jul  7 12:38 tools/perf/util/bpf_skel/sample_filter.bpf.c
[acme@nine linux]$

There are 'perf test' entries for the BPF support in perf that is pre
libbpf skel functionality above and as well some for the libbpf based
skel features, such as:

[root@quaco ~]# perf test -vvv contention
 87: kernel lock contention analysis test                            :
--- start ---
test child forked, pid 213314
Testing perf lock record and perf lock contention
Testing perf lock contention --use-bpf
Testing perf lock record and perf lock contention at the same time
Testing perf lock contention --threads
Testing perf lock contention --lock-addr
Testing perf lock contention --type-filter (w/ spinlock)
Testing perf lock contention --lock-filter (w/ tasklist_lock)
Testing perf lock contention --callstack-filter (w/ unix_stream)
Testing perf lock contention --callstack-filter with task aggregation
Testing perf lock contention CSV output
test child finished with 0
---- end ----
kernel lock contention analysis test: Ok
[root@quaco ~]#

We could perhaps add support for some kind of tags to the tests, to
help run just the ones that use bpf and then you could use:

 # perf test --uses bpf

At first we would just add this tag, then we would use it for other
stuff as the need arises to run just a subset of the tests.

But for now you could start perhaps with:

[root@quaco ~]# perf test LLVM ; perf test "BPF " ; perf test "lock contention"
 40: LLVM search and compile                                         :
 40.1: Basic BPF llvm compile                                        : Ok
 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                                       : FAILED!
 87: kernel lock contention analysis test                            : Ok
[root@quaco ~]#

- Arnaldo

Re: [BUG] perf test: Regression because of d6e6286a12e7

[Alexei Starovoitov]

On Wed, Jul 12, 2023 at 8:39 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Right, perhaps the libbpf CI could try building perf, preferably with
> BUILD_BPF_SKEL=1, to enable these tools:


That would be great.
perf experts probably should do pull-req to bpf CI to enable that.
See slides:
http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-bpf-ci.pdf

"How to contribute?
Depending on what part of CI you are changing, you can create a pull request to
https://github.com/kernel-patches/vmtest/
https://github.com/libbpf/ci
"

Re: [BUG] perf test: Regression because of d6e6286a12e7

[Arnaldo Carvalho de Melo]

Em Wed, Jul 12, 2023 at 11:20:27AM -0700, Alexei Starovoitov escreveu:
> On Wed, Jul 12, 2023 at 8:39 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Right, perhaps the libbpf CI could try building perf, preferably with
> > BUILD_BPF_SKEL=1, to enable these tools:
> 
> 
> That would be great.
> perf experts probably should do pull-req to bpf CI to enable that.
> See slides:
> http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-bpf-ci.pdf
> 
> "How to contribute?
> Depending on what part of CI you are changing, you can create a pull request to
> https://github.com/kernel-patches/vmtest/
> https://github.com/libbpf/ci
> "

Sure, I still recall Quentin's talk about CI, etc in Dublin, will come
up with something and submit.

- Arnaldo

Re: [BUG] perf test: Regression because of d6e6286a12e7

[Mykola Lysenko]

Hey Arnaldo,

> On Jul 13, 2023, at 1:57 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> !-------------------------------------------------------------------|
>  This Message Is From an External Sender
> 
> |-------------------------------------------------------------------!
> 
> Em Wed, Jul 12, 2023 at 11:20:27AM -0700, Alexei Starovoitov escreveu:
>> On Wed, Jul 12, 2023 at 8:39 AM Arnaldo Carvalho de Melo
>> <acme@kernel.org> wrote:
>>> 
>>> Right, perhaps the libbpf CI could try building perf, preferably with
>>> BUILD_BPF_SKEL=1, to enable these tools:
>> 
>> 
>> That would be great.
>> perf experts probably should do pull-req to bpf CI to enable that.
>> See slides:
>> http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-bpf-ci.pdf
>> 
>> "How to contribute?
>> Depending on what part of CI you are changing, you can create a pull request to
>> https://github.com/kernel-patches/vmtest/
>> https://github.com/libbpf/ci
>> "
> 
> Sure, I still recall Quentin's talk about CI, etc in Dublin, will come
> up with something and submit.

Thanks for looking at this!

If you will have any questions on how CI works, do not hesitate to join BPF office hours and we will do our best to answer.

Mykola

> 
> - Arnaldo

Re: [BUG] perf test: Regression because of d6e6286a12e7

[Ian Rogers]

On Thu, Sep 14, 2023 at 2:43 PM Manu Bretelle <chantra@meta.com> wrote:
>
> Hi Arnaldo,
>
>
>
> Checking back here to see if there is anything you need help in order to add perf support to BPF CI. Were you able to make progress and are hitting some issues along the way?
>

Separate from the CI issue there were some updates on perf:
 - we're looking to re-enable BPF skeletons by default for 6.7:
https://lore.kernel.org/lkml/20230914211948.814999-1-irogers@google.com/
 - Ravi's original failure can no longer fail as we removed the BPF
filter events in favor of a BPF skeleton based --filter option:
https://lore.kernel.org/lkml/20230810184853.2860737-1-irogers@google.com/
As such these tests no longer exist. Other tests like kernel lock
contention analysis implicitly use BPF and so we are still testing
BPF.

Thanks,
Ian

>
> Thanks,
>
>
>
> Manu
>
>
>
> From: Mykola Lysenko <mykolal@meta.com>
> Date: Friday, July 14, 2023 at 11:15 AM
> To: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Mykola Lysenko <mykolal@meta.com>, Alexei Starovoitov <alexei.starovoitov@gmail.com>, Ravi Bangoria <ravi.bangoria@amd.com>, Andrii Nakryiko <andrii.nakryiko@gmail.com>, Andrii Nakryiko <andrii@kernel.org>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, Ian Rogers <irogers@google.com>, linux-perf-users <linux-perf-users@vger.kernel.org>, bpf <bpf@vger.kernel.org>, Manu Bretelle <chantra@meta.com>, Daniel Müller <deso@posteo.net>, Mykola Lysenko <mykolal@meta.com>
> Subject: Re: [BUG] perf test: Regression because of d6e6286a12e7
>
> Hey Arnaldo,
>
> > On Jul 13, 2023, at 1:57 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> >
> >
> > Em Wed, Jul 12, 2023 at 11:20:27AM -0700, Alexei Starovoitov escreveu:
> >> On Wed, Jul 12, 2023 at 8:39 AM Arnaldo Carvalho de Melo
> >> <acme@kernel.org> wrote:
> >>>
> >>> Right, perhaps the libbpf CI could try building perf, preferably with
> >>> BUILD_BPF_SKEL=1, to enable these tools:
> >>
> >>
> >> That would be great.
> >> perf experts probably should do pull-req to bpf CI to enable that.
> >> See slides:
> >> http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-bpf-ci.pdf
> >>
> >> "How to contribute?
> >> Depending on what part of CI you are changing, you can create a pull request to
> >> https://github.com/kernel-patches/vmtest/
> >> https://github.com/libbpf/ci
> >> "
> >
> > Sure, I still recall Quentin's talk about CI, etc in Dublin, will come
> > up with something and submit.
>
> Thanks for looking at this!
>
> If you will have any questions on how CI works, do not hesitate to join BPF office hours and we will do our best to answer.
>
> Mykola
>
> >
> > - Arnaldo
>

Re: [BUG] perf test: Regression because of d6e6286a12e7

[Arnaldo Carvalho de Melo]

Em Thu, Sep 14, 2023 at 09:43:41PM +0000, Manu Bretelle escreveu:
> Hi Arnaldo,
 
> Checking back here to see if there is anything you need help in order
> to add perf support to BPF CI. Were you able to make progress and are
> hitting some issues along the way?

I'm travelling now, but I saw that Ian replied.

No progress on my part, but I would start with a simple:

Build perf from the same kernel source tree used in the
current CI tests, then run 'perf test'. It would test more than just the
BPF part, but would be a good start.

A second step would be just disabling the tests that are failing and
that are not BPF specific, using the --skip arg to 'perf test':

[root@quaco test]# perf test -h skip

 Usage: perf test [<options>] [{list <test-name-fragment>|[<test-name-fragments>|<test-numbers>]}]

    -s, --skip <tests>    tests to skip

[root@quaco test]#

So something like:

-----------------
make -C tools/perf
perf test --skip list,of,tests,failing,that,are,not,BPF,specific
-----------------

The perf build dependencies should be similar to the ones needed to
build libbpf or close to it, so the above would be a great first stab at
it.

- Arnaldo
 
> 
> Thanks,
> 
> Manu
> 
> From: Mykola Lysenko <mykolal@meta.com>
> Date: Friday, July 14, 2023 at 11:15 AM
> To: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Mykola Lysenko <mykolal@meta.com>, Alexei Starovoitov <alexei.starovoitov@gmail.com>, Ravi Bangoria <ravi.bangoria@amd.com>, Andrii Nakryiko <andrii.nakryiko@gmail.com>, Andrii Nakryiko <andrii@kernel.org>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, Ian Rogers <irogers@google.com>, linux-perf-users <linux-perf-users@vger.kernel.org>, bpf <bpf@vger.kernel.org>, Manu Bretelle <chantra@meta.com>, Daniel Müller <deso@posteo.net>, Mykola Lysenko <mykolal@meta.com>
> Subject: Re: [BUG] perf test: Regression because of d6e6286a12e7
> Hey Arnaldo,
> 
> > On Jul 13, 2023, at 1:57 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> > !-------------------------------------------------------------------|
> >  This Message Is From an External Sender
> >
> > |-------------------------------------------------------------------!
> >
> > Em Wed, Jul 12, 2023 at 11:20:27AM -0700, Alexei Starovoitov escreveu:
> >> On Wed, Jul 12, 2023 at 8:39 AM Arnaldo Carvalho de Melo
> >> <acme@kernel.org> wrote:
> >>>
> >>> Right, perhaps the libbpf CI could try building perf, preferably with
> >>> BUILD_BPF_SKEL=1, to enable these tools:
> >>
> >>
> >> That would be great.
> >> perf experts probably should do pull-req to bpf CI to enable that.
> >> See slides:
> >> http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-bpf-ci.pdf
> >>
> >> "How to contribute?
> >> Depending on what part of CI you are changing, you can create a pull request to
> >> https://github.com/kernel-patches/vmtest/
> >> https://github.com/libbpf/ci
> >> "
> >
> > Sure, I still recall Quentin's talk about CI, etc in Dublin, will come
> > up with something and submit.
> 
> Thanks for looking at this!
> 
> If you will have any questions on how CI works, do not hesitate to join BPF office hours and we will do our best to answer.
> 
> Mykola
> 
> >
> > - Arnaldo
> 

-- 

- Arnaldo