On Tue, Apr 28, 2020 at 05:33:54PM -0700, Yonghong Song wrote:
>
>
> On 4/28/20 5:29 PM, Song Liu 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
> > Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
...
> > +static int val = 1;
> > +
> > +SEC("raw_tracepoint/sys_enter")
> > +int test_enable_stats(void *ctx)
> > +{
> > + __u32 key = 0;
> > + __u64 *val;
>
> The above two declarations (key/val) are not needed,
> esp. "val" is shadowing.
> Maybe the maintainer can fix it up before merging
> if there is no other changes for this patch set.
>
> > +
> > + val += 1;
I think 'PASSED' above is quite misleading.
How it can pass when it wasn't incremented?
The user space test_enable_stats() doesn't check this val.
Please fix.
usleep(1000); needs an explanation as well.
Why 1000 ? It should work with any syscall. like getpid ?
and with value 1 ?
Since there is bpf_obj_get_info_by_fd() that usleep()
is unnecessary. What am I missing?
