On Wed, Apr 29, 2020 at 10:12 PM Song Liu <songliubraving@xxxxxx> wrote:
>
>
>
> > On Apr 29, 2020, at 7:23 PM, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > On Tue, Apr 28, 2020 at 11:47 PM Song Liu <songliubraving@xxxxxx> wrote:
> >>
> >> Add test for BPF_ENABLE_STATS, which should enable run_time_ns stats.
> >>
> >> ~/selftests/bpf# ./test_progs -t enable_stats -v
> >> test_enable_stats:PASS:skel_open_and_load 0 nsec
> >> test_enable_stats:PASS:get_stats_fd 0 nsec
> >> test_enable_stats:PASS:attach_raw_tp 0 nsec
> >> test_enable_stats:PASS:get_prog_info 0 nsec
> >> test_enable_stats:PASS:check_stats_enabled 0 nsec
> >> test_enable_stats:PASS:check_run_cnt_valid 0 nsec
> >> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> >>
> >> Signed-off-by: Song Liu <songliubraving@xxxxxx>
> >> ---
> >> .../selftests/bpf/prog_tests/enable_stats.c | 46 +++++++++++++++++++
> >> .../selftests/bpf/progs/test_enable_stats.c | 18 ++++++++
> >> 2 files changed, 64 insertions(+)
> >> create mode 100644 tools/testing/selftests/bpf/prog_tests/enable_stats.c
> >> create mode 100644 tools/testing/selftests/bpf/progs/test_enable_stats.c
> >>
> >> diff --git a/tools/testing/selftests/bpf/prog_tests/enable_stats.c b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> >> new file mode 100644
> >> index 000000000000..cb5e34dcfd42
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> >> @@ -0,0 +1,46 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +#include <test_progs.h>
> >> +#include <sys/mman.h>
> >
> > is this header used for anything?
>
> Not really, will remove it.
>
> >
> >> +#include "test_enable_stats.skel.h"
> >> +
> >> +void test_enable_stats(void)
> >> +{
> >
> > [...]
> >
> >> +
> >> +char _license[] SEC("license") = "GPL";
> >> +
> >> +static __u64 count;
> >
> > this is actually very unreliable, because compiler might decide to
> > just remove this variable. It should be either `static volatile`, or
> > better use zero-initialized global variable:
> >
> > __u64 count = 0;
>
> Why would compile remove it? Is it because "static" or "no initialized?
because static, which makes compiler assume that no one else can
access it (which is not true for BPF programs).
> Would "__u64 count;" work?
unfortunately, no, libbpf enforces that all global variables are
initialized (uninitialized global variables go into special COM
section, libbpf doesn't support it).
>
> For "__u64 count = 0;", checkpatch.pl generates an error:
>
> ERROR: do not initialise globals to 0
> #92: FILE: tools/testing/selftests/bpf/progs/test_enable_stats.c:11:
> +__u64 count = 0;
ignore checkpatch.pl in this case?
>
> Thanks,
> Song
