All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Jiri Olsa <olsajiri@gmail.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	martin.lau@kernel.org, kernel-team@meta.com
Subject: Re: [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized -O2 mode
Date: Thu, 5 Oct 2023 09:19:26 +0200	[thread overview]
Message-ID: <ZR5jfh939hHLtnED@krava> (raw)
In-Reply-To: <CAEf4Bzag+5_r05t7p2N-XiWykT51U5x4ov7YSa0NGVJrGpo6UQ@mail.gmail.com>

On Wed, Oct 04, 2023 at 10:21:12AM -0700, Andrii Nakryiko wrote:
> On Wed, Oct 4, 2023 at 1:27 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Tue, Oct 03, 2023 at 05:17:49PM -0700, Andrii Nakryiko wrote:
> > > Add support for building selftests with -O2 level of optimization, which
> > > allows more compiler warnings detection (like lots of potentially
> > > uninitialized usage), but also is useful to have a faster-running test
> > > for some CPU-intensive tests.
> > >
> > > One can build optimized versions of libbpf and selftests by running:
> > >
> > >   $ make RELEASE=1
> > >
> > > There is a measurable speed up of about 10 seconds for me locally,
> > > though it's mostly capped by non-parallelized serial tests. User CPU
> > > time goes down by total 40 seconds, from 1m10s to 0m28s.
> > >
> > > Unoptimized build (-O0)
> > > =======================
> > > Summary: 430/3544 PASSED, 25 SKIPPED, 4 FAILED
> > >
> > > real    1m59.937s
> > > user    1m10.877s
> > > sys     3m14.880s
> > >
> > > Optimized build (-O2)
> > > =====================
> > > Summary: 425/3543 PASSED, 25 SKIPPED, 9 FAILED
> > >
> > > real    1m50.540s
> > > user    0m28.406s
> > > sys     3m13.198s
> >
> > hi,
> > I get following error when running selftest compiled with RELEASE=1
> >
> > # ./test_progs -t attach_probe/manual-legacy
> > test_attach_probe:PASS:skel_open 0 nsec
> > test_attach_probe:PASS:skel_load 0 nsec
> > test_attach_probe:PASS:check_bss 0 nsec
> > test_attach_probe:PASS:uprobe_ref_ctr_cleanup 0 nsec
> > test_attach_probe_manual:PASS:skel_kprobe_manual_open_and_load 0 nsec
> > test_attach_probe_manual:PASS:uprobe_offset 0 nsec
> > test_attach_probe_manual:PASS:attach_kprobe 0 nsec
> > test_attach_probe_manual:PASS:attach_kretprobe 0 nsec
> > test_attach_probe_manual:PASS:attach_uprobe 0 nsec
> > test_attach_probe_manual:PASS:attach_uretprobe 0 nsec
> > libbpf: failed to add legacy uprobe event for /proc/self/exe:0x19020: -17
> > libbpf: prog 'handle_uprobe_byname': failed to create uprobe '/proc/self/exe:0x19020' perf event: File exists
> > test_attach_probe_manual:FAIL:attach_uprobe_byname unexpected error: -17
> > #8/2     attach_probe/manual-legacy:FAIL
> > #8       attach_probe:FAIL
> >
> >
> > it looks like -O2 can merge some of the trigger functions:
> >
> >         [root@qemu bpf]# nm test_progs | grep trigger_func
> >         0000000000558f30 t autoattach_trigger_func.constprop.0
> >         000000000041d240 t trigger_func
> >         0000000000419020 t trigger_func
> >         0000000000420e70 t trigger_func
> >         0000000000507aa0 t trigger_func
> >         0000000000419020 t trigger_func2
> >         0000000000419020 t trigger_func3
> >         0000000000419030 t trigger_func4
> >         [root@qemu bpf]# nm test_progs | grep 0000000000419020
> >         0000000000419020 t trigger_func
> >         0000000000419020 t trigger_func2
> >         0000000000419020 t trigger_func3
> >
> > I got more tests fails, but I suspect it's all for similar
> > reason like above
> >
> 
> yes, I didn't say that -O2 version passes all tests :) at least there
> are complicated USDT cases under -O2 which libbpf can't support (if I
> remember correctly, it was offset relative to global symbol case). But
> it's the first step. And once we have ability to build with RELEASE=1,
> we can add it as a separate test in CI and catch more of these
> uninitialized usage errors. Initially we can denylist tests that are
> broken due to -O2 and work to fix them.

I see ;-) sounds good

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> 
> > jirka
> >
> >
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > ---
> > >  tools/testing/selftests/bpf/Makefile | 14 ++++++++------
> > >  1 file changed, 8 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index a25e262dbc69..55d1b1848e6c 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -27,7 +27,9 @@ endif
> > >  BPF_GCC              ?= $(shell command -v bpf-gcc;)
> > >  SAN_CFLAGS   ?=
> > >  SAN_LDFLAGS  ?= $(SAN_CFLAGS)
> > > -CFLAGS += -g -O0 -rdynamic                                           \
> > > +RELEASE              ?=
> > > +OPT_FLAGS    ?= $(if $(RELEASE),-O2,-O0)
> > > +CFLAGS += -g $(OPT_FLAGS) -rdynamic                                  \
> > >         -Wall -Werror                                                 \
> > >         $(GENFLAGS) $(SAN_CFLAGS)                                     \
> > >         -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR)          \
> > > @@ -241,7 +243,7 @@ $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT)
> > >                   BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/                  \
> > >                   BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf                          \
> > >                   BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR)                \
> > > -                 EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)'                        \
> > > +                 EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)'               \
> > >                   EXTRA_LDFLAGS='$(SAN_LDFLAGS)' &&                          \
> > >                   cp $(RUNQSLOWER_OUTPUT)runqslower $@
> > >
> > > @@ -279,7 +281,7 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)    \
> > >                   $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
> > >       $(Q)$(MAKE) $(submake_extras)  -C $(BPFTOOLDIR)                        \
> > >                   ARCH= CROSS_COMPILE= CC="$(HOSTCC)" LD="$(HOSTLD)"         \
> > > -                 EXTRA_CFLAGS='-g -O0'                                      \
> > > +                 EXTRA_CFLAGS='-g $(OPT_FLAGS)'                             \
> > >                   OUTPUT=$(HOST_BUILD_DIR)/bpftool/                          \
> > >                   LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/                    \
> > >                   LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/                        \
> > > @@ -290,7 +292,7 @@ $(CROSS_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> > >                   $(BPFOBJ) | $(BUILD_DIR)/bpftool
> > >       $(Q)$(MAKE) $(submake_extras)  -C $(BPFTOOLDIR)                         \
> > >                   ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE)                 \
> > > -                 EXTRA_CFLAGS='-g -O0'                                       \
> > > +                 EXTRA_CFLAGS='-g $(OPT_FLAGS)'                              \
> > >                   OUTPUT=$(BUILD_DIR)/bpftool/                                \
> > >                   LIBBPF_OUTPUT=$(BUILD_DIR)/libbpf/                          \
> > >                   LIBBPF_DESTDIR=$(SCRATCH_DIR)/                              \
> > > @@ -313,7 +315,7 @@ $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)                       \
> > >          $(APIDIR)/linux/bpf.h                                               \
> > >          | $(BUILD_DIR)/libbpf
> > >       $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
> > > -                 EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)'                        \
> > > +                 EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)'               \
> > >                   EXTRA_LDFLAGS='$(SAN_LDFLAGS)'                             \
> > >                   DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
> > >
> > > @@ -322,7 +324,7 @@ $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)                  \
> > >               $(APIDIR)/linux/bpf.h                                          \
> > >               | $(HOST_BUILD_DIR)/libbpf
> > >       $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR)                             \
> > > -                 EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE=                 \
> > > +                 EXTRA_CFLAGS='-g $(OPT_FLAGS)' ARCH= CROSS_COMPILE=        \
> > >                   OUTPUT=$(HOST_BUILD_DIR)/libbpf/                           \
> > >                   CC="$(HOSTCC)" LD="$(HOSTLD)"                              \
> > >                   DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
> > > --
> > > 2.34.1
> > >
> > >

  reply	other threads:[~2023-10-05  7:19 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-04  0:17 [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Andrii Nakryiko
2023-10-04  0:17 ` [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized " Andrii Nakryiko
2023-10-04  8:27   ` Jiri Olsa
2023-10-04 17:21     ` Andrii Nakryiko
2023-10-05  7:19       ` Jiri Olsa [this message]
2023-10-05  9:04         ` Alan Maguire
2023-10-06 17:55           ` Andrii Nakryiko
2023-10-06 17:56             ` Andrii Nakryiko
2023-10-04  0:17 ` [PATCH bpf-next 3/3] selftests/bpf: don't truncate #test/subtest field Andrii Nakryiko
2023-10-05  7:21   ` Jiri Olsa
2023-10-05  7:21 ` [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Jiri Olsa
2023-10-06 18:20 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZR5jfh939hHLtnED@krava \
    --to=olsajiri@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@meta.com \
    --cc=martin.lau@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.