On Fri, Jun 24, 2022 at 10:22:38AM -0700, Martin KaFai Lau wrote:
> On Thu, Jun 23, 2022 at 04:46:09PM -0700, Dave Marchevsky wrote:
> > +static void report_progress(int iter, struct bench_res *res, long delta_ns)
> > +{
> > + if (ctx.skel->bss->unexpected) {
> > + fprintf(stderr, "Error: Unexpected order of bpf prog calls (postgp after pregp).");
> > + fprintf(stderr, "Data can't be trusted, exiting\n");
> > + exit(1);
> > + }
> > +
> > + if (args.quiet)
> > + return;
> > +
> > + printf("Iter %d\t avg tasks_trace grace period latency\t%lf ns\n",
> > + iter, res->gp_ns / (double)res->gp_ct);
> > + printf("Iter %d\t avg ticks per tasks_trace grace period\t%lf\n",
> > + iter, res->stime / (double)res->gp_ct);
> > +}
> > +
>
> [ ... ]
>
> > diff --git a/tools/testing/selftests/bpf/benchs/run_bench_local_storage_rcu_tasks_trace.sh b/tools/testing/selftests/bpf/benchs/run_bench_local_storage_rcu_tasks_trace.sh
> > new file mode 100755
> > index 000000000000..5dac1f02892c
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/benchs/run_bench_local_storage_rcu_tasks_trace.sh
> > @@ -0,0 +1,11 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +kthread_pid=`pgrep rcu_tasks_trace_kthread`
> > +
> > +if [ -z $kthread_pid ]; then
> > + echo "error: Couldn't find rcu_tasks_trace_kthread"
> > + exit 1
> > +fi
> > +
> > +./bench --nr_procs 15000 --kthread_pid $kthread_pid -d 600 --quiet 1 local-storage-tasks-trace
> > diff --git a/tools/testing/selftests/bpf/progs/local_storage_rcu_tasks_trace_bench.c b/tools/testing/selftests/bpf/progs/local_storage_rcu_tasks_trace_bench.c
> > new file mode 100644
> > index 000000000000..9b11342b19a0
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/local_storage_rcu_tasks_trace_bench.c
> > @@ -0,0 +1,65 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */
> > +
> > +#include "vmlinux.h"
> > +#include <bpf/bpf_helpers.h>
> > +#include "bpf_misc.h"
> > +
> > +struct {
> > + __uint(type, BPF_MAP_TYPE_TASK_STORAGE);
> > + __uint(map_flags, BPF_F_NO_PREALLOC);
> > + __type(key, int);
> > + __type(value, int);
> > +} task_storage SEC(".maps");
> > +
> > +long hits;
> > +long gp_hits;
> > +long gp_times;
> > +long current_gp_start;
> > +long unexpected;
> > +
> > +SEC("fentry/" SYS_PREFIX "sys_getpgid")
> > +int get_local(void *ctx)
> > +{
> > + struct task_struct *task;
> > + int idx;
> > + int *s;
> > +
> > + idx = 0;
> > + task = bpf_get_current_task_btf();
> > + s = bpf_task_storage_get(&task_storage, task, &idx,
> > + BPF_LOCAL_STORAGE_GET_F_CREATE);
> > + if (!s)
> > + return 0;
> > +
> > + *s = 3;
> > + bpf_task_storage_delete(&task_storage, task);
> > + __sync_add_and_fetch(&hits, 1);
> > + return 0;
> > +}
> > +
> > +SEC("kprobe/rcu_tasks_trace_pregp_step")
> nit. Similar to the fentry sys_getpgid above.
> may as well use fentry for everything.
>
> > +int pregp_step(struct pt_regs *ctx)
> > +{
> > + current_gp_start = bpf_ktime_get_ns();
> > + return 0;
> > +}
> > +
> > +SEC("kprobe/rcu_tasks_trace_postgp")
> > +int postgp(struct pt_regs *ctx)
> > +{
> > + if (!current_gp_start) {
> > + /* Will only happen if prog tracing rcu_tasks_trace_pregp_step doesn't
> > + * execute before this prog
> > + */
> > + __sync_add_and_fetch(&unexpected, 1);
> I consistently hit this:
> ./bench --nr_procs 1500 --kthread_pid ... -d 60 --quiet 1 local-storage-tasks-trace
> Setting up benchmark 'local-storage-tasks-trace'...
> Spun up 1500 procs (our pid 28351)
> Benchmark 'local-storage-tasks-trace' started.
> Error: Unexpected order of bpf prog calls (postgp after pregp).Data can't be trusted, exiting
>
> May be there is a chance for the very first postgp being called
> before the pregp_step?
If the test starts in the midst of a grace period, yes, this can
happen. Can the code ignore a postgp that is not preceded by
a pregp?
> Thanks for working on this!
What Martin said!
Thanx, Paul
