On Tue, Jan 31, 2023 at 2:57 AM Anton Protopopov <aspsk@xxxxxxxxxxxxx> wrote:
>
> On 23/01/30 04:10, Andrii Nakryiko wrote:
> > On Fri, Jan 27, 2023 at 10:14 AM Anton Protopopov <aspsk@xxxxxxxxxxxxx> wrote:
> > >
> > > The "local-storage-tasks-trace" benchmark has a `--quiet` option. Move it to
> > > the list of common options, so that the main code and other benchmarks can use
> > > (now global) env.quiet as well.
> > >
> > > Signed-off-by: Anton Protopopov <aspsk@xxxxxxxxxxxxx>
> > > ---
> > > tools/testing/selftests/bpf/bench.c | 15 +++++++++++++++
> > > tools/testing/selftests/bpf/bench.h | 1 +
> > > .../benchs/bench_local_storage_rcu_tasks_trace.c | 14 +-------------
> > > 3 files changed, 17 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c
> > > index ba93f1b268e1..42bf41a9385e 100644
> > > --- a/tools/testing/selftests/bpf/bench.c
> > > +++ b/tools/testing/selftests/bpf/bench.c
> > > @@ -16,6 +16,7 @@ struct env env = {
> > > .warmup_sec = 1,
> > > .duration_sec = 5,
> > > .affinity = false,
> > > + .quiet = false,
> > > .consumer_cnt = 1,
> > > .producer_cnt = 1,
> > > };
> > > @@ -257,6 +258,7 @@ static const struct argp_option opts[] = {
> > > { "consumers", 'c', "NUM", 0, "Number of consumer threads"},
> > > { "verbose", 'v', NULL, 0, "Verbose debug output"},
> > > { "affinity", 'a', NULL, 0, "Set consumer/producer thread affinity"},
> > > + { "quiet", 'q', "{0,1}", OPTION_ARG_OPTIONAL, "If true, be quiet"},
> >
> > given the default is not quiet, why add 0 or 1? -q for quiet, no "-q"
> > for not quiet? Keeping it simple?
>
> The local-storage-tasks-trace benchmark expected 0 or 1 there, so I didn't want
> to break any script which utilize this option.
>
> The new parser accepts the old --quiet=0|1 for consistency, but also -q|--quiet
> without value, as you've suggested (I pass OPTION_ARG_OPTIONAL and set
> quiet=true if arg is NULL in the new parser).
>
I think it was mostly due to copy/pasting some other integer-based
argument handling. We don't need that for boolean flags. Let's just
fix benchs/run_bench_local_storage_rcu_tasks_trace.sh to do -q and
keep it simple?
> > > { "prod-affinity", ARG_PROD_AFFINITY_SET, "CPUSET", 0,
> > > "Set of CPUs for producer threads; implies --affinity"},
> > > { "cons-affinity", ARG_CONS_AFFINITY_SET, "CPUSET", 0,
> >
> > [...]
