Re: [PATCH 05/12] perf tools: Allow BPF program config probing options

[Wangnan (F)]

On 2015/11/13 23:46, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 13, 2015 at 12:29:14PM +0000, Wang Nan escreveu:
> > By extending the syntax of BPF object section names, this patch allows
> > user to config probing options like what they can do in 'perf probe'.
> >
> > Test result:
> >
> > For following BPF file bpf.c:
> >
> >   SEC("inlines=no\n"
> >       "func=SyS_dup?")
> >   int func(void *ctx)
> >   {
> >   	return 1;
> >   }
> >
> >   Cmdline:
> >
> >   # ./perf record  -e ./test_probe_glob.c ls /
> >   ...
> >   [ perf record: Woken up 1 times to write data ]
> >   [ perf record: Captured and wrote 0.013 MB perf.data ]
> >   # ./perf evlist
> >   perf_bpf_probe:func_1
> >   perf_bpf_probe:func
> >
> > Change "inlines=no" to "inlines=yes":
> >
> > Cmdline:
> >
> >   # ./perf record  -e ./test_probe_glob.c ls /
> >   ...
> >   [ perf record: Woken up 2 times to write data ]
> >   [ perf record: Captured and wrote 0.013 MB perf.data ]
> >   # ./perf evlist
> >   perf_bpf_probe:func_3
> >   perf_bpf_probe:func_2
> >   perf_bpf_probe:func_1
> >   perf_bpf_probe:func
> >
> > Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
> > Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
> > Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> > Cc: Zefan Li <lizefan@xxxxxxxxxx>
> > Cc: pi3orama@xxxxxxx
> > ---

[SNIP]

> > diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> > index 47b1e36..55785d5 100644
> > --- a/tools/perf/util/util.c
> > +++ b/tools/perf/util/util.c
> > @@ -695,3 +695,21 @@ fetch_kernel_version(unsigned int *puint, char *str,
> >   		*puint = (version << 16) + (patchlevel << 8) + sublevel;
> >   	return 0;
> >   }
> > +
> > +int convert_str_to_bool(const char *str, bool *result)
> strtobool() should be more compact and convey the same idea....
>
> Hey, I googled for that name and guess what, the kernel has exactly this
> function:
>
> lib/string.c
>
> /**
>   * strtobool - convert common user inputs into boolean values
>   * @s: input string
>   * @res: result
>   *
>   * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
>   * Otherwise it will return -EINVAL.  Value pointed to by res is
>   * updated upon finding a match.
>   */
>
> include/linux/string.h
>
> So, please add it to tools/include/linux/string.h and
> tools/lib/util/string.c, this way we use the same code as the kernel,
> with the same function signature, etc.

Good suggestion. One small inconvenience: kernel's strtobool
only support 'yes|no', it doesn't match perf's config, which
requires parsing yes/no/on/off/true/false. In addition, it
accept '0/1' but perf config not.

It is very easy for me to change strtobool() to match them.
However, do you think we can provide a function with same
name with kernel but have different behavior?

However I think we can first clone that function and use it in
BPF without touching perf config, then decide whether we need
to change strtobool() or not.

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/