Re: [PATCH bpf-next 4/6] selftest/bpf/benchs: make quiet option common

[Anton Protopopov <aspsk@isovalent.com>]

On 23/01/30 04:10, Andrii Nakryiko wrote:
> On Fri, Jan 27, 2023 at 10:14 AM Anton Protopopov <aspsk@isovalent.com> 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@isovalent.com>
> > ---
> >  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).

> >         { "prod-affinity", ARG_PROD_AFFINITY_SET, "CPUSET", 0,
> >           "Set of CPUs for producer threads; implies --affinity"},
> >         { "cons-affinity", ARG_CONS_AFFINITY_SET, "CPUSET", 0,
> 
> [...]