Re: [PATCH v6 1/4] perf check: introduce check subcommand

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Nov 10, 2023 at 6:38 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Fri, Nov 10, 2023 at 11:31:57AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Fri, Nov 10, 2023 at 11:27:42AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Sat, Oct 21, 2023 at 08:35:23PM +0530, Aditya Gupta escreveu:
> > > > Acked-by: Namhyung Kim <namhyung@kernel.org>
> > > > Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
>
> > > Right after applying this first patch:
>
> > > [acme@quaco perf-tools-next]$ m
> > > make: Entering directory '/home/acme/git/perf-tools-next/tools/perf'
> > > builtin-check.c:53:56: error: expected ‘}’ before ‘;’ token
> > >    53 |         FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL);
> > >       |                                                        ^
> > > builtin-check.c:29:47: note: to match this ‘{’
> > >    29 | struct feature_support supported_features[] = {
> > >       |                                               ^
> > > make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:106: /tmp/build/perf-tools-next/builtin-check.o] Error 1
> > > make[3]: *** Waiting for unfinished jobs....
> > > make[2]: *** [Makefile.perf:669: /tmp/build/perf-tools-next/perf-in.o] Error 2
> > > make[1]: *** [Makefile.perf:242: sub-make] Error 2
> > > make: *** [Makefile:113: install-bin] Error 2
> > > make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
>
> > It was a simple error, please be more careful next time.
>
> > I'm testing the rest of the patchset now.
>
> Please resubmit when you address the problem fixed with this patch and:
>
> Mussing newline, why print the version again? What is that "perf check "
> prefix for?
>
> [acme@quaco perf-tools-next]$ perf check --feature traceevent
> perf check 6.6.rc1.g78fa196349bc
> Feature not known: traceevent[acme@quaco perf-tools-next]$
>
> Why should we require "--feature"? The following format is descriptive
> enough:

I thought we might want to extend the functionality of the check
command later.  Maybe we can check how many perf counters
are supported or which sysctl settings are affected and so on.

Maybe we need to make '--feature' option to a sub-command
like `perf check feature xxx`.  So it cannot be mixed with others
later.

>
> [acme@quaco perf-tools-next]$
> [acme@quaco perf-tools-next]$ perf check traceevent
> perf check 6.6.rc1.g78fa196349bc
> [acme@quaco perf-tools-next]$
>
> So I had to go back and use:
>
> [acme@quaco perf-tools-next]$ perf -vv
> perf version 6.6.rc1.g78fa196349bc
>                  dwarf: [ on  ]  # HAVE_DWARF_SUPPORT
>     dwarf_getlocations: [ on  ]  # HAVE_DWARF_GETLOCATIONS_SUPPORT
>          syscall_table: [ on  ]  # HAVE_SYSCALL_TABLE_SUPPORT
>                 libbfd: [ OFF ]  # HAVE_LIBBFD_SUPPORT
>             debuginfod: [ on  ]  # HAVE_DEBUGINFOD_SUPPORT
>                 libelf: [ on  ]  # HAVE_LIBELF_SUPPORT
>                libnuma: [ on  ]  # HAVE_LIBNUMA_SUPPORT
> numa_num_possible_cpus: [ on  ]  # HAVE_LIBNUMA_SUPPORT
>                libperl: [ on  ]  # HAVE_LIBPERL_SUPPORT
>              libpython: [ on  ]  # HAVE_LIBPYTHON_SUPPORT
>               libslang: [ on  ]  # HAVE_SLANG_SUPPORT
>              libcrypto: [ on  ]  # HAVE_LIBCRYPTO_SUPPORT
>              libunwind: [ on  ]  # HAVE_LIBUNWIND_SUPPORT
>     libdw-dwarf-unwind: [ on  ]  # HAVE_DWARF_SUPPORT
>                   zlib: [ on  ]  # HAVE_ZLIB_SUPPORT
>                   lzma: [ on  ]  # HAVE_LZMA_SUPPORT
>              get_cpuid: [ on  ]  # HAVE_AUXTRACE_SUPPORT
>                    bpf: [ on  ]  # HAVE_LIBBPF_SUPPORT
>                    aio: [ on  ]  # HAVE_AIO_SUPPORT
>                   zstd: [ on  ]  # HAVE_ZSTD_SUPPORT
>                libpfm4: [ on  ]  # HAVE_LIBPFM
>          libtraceevent: [ on  ]  # HAVE_LIBTRACEEVENT
>          bpf_skeletons: [ on  ]  # HAVE_BPF_SKEL
> [acme@quaco perf-tools-next]$
>
> To see (some) of the features, please add a:
>
> # perf check --list-features
>
> So that we can get that features array printed.
>
> Some other feature requests:
>
> [acme@quaco perf-tools-next]$ perf check --feature libtraceevent,libbpf_support
> perf check 6.6.rc1.g78fa196349bc
> Feature not known: libtraceevent,libbpf_support[acme@quaco perf-tools-next]$
>
> This should return true if both are available.

Sounds good.

>
> Also, shouldn't be --quiet be the default since this is being designed
> for use in scripts?

I don't know.. users might want to see the result instead
of checking the return value.

     libtraceevent: [ on  ]
    libbpf_support: [ OFF ]

Thanks,
Namhyung