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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

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:

[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.

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

- Arnaldo
 
> - Arnaldo
> 
> diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
> index 1183983e4352798d..1502804780507b5d 100644
> --- a/tools/perf/builtin-check.c
> +++ b/tools/perf/builtin-check.c
> @@ -50,7 +50,7 @@ struct feature_support supported_features[] = {
>  	FEATURE_SUPPORT("zstd", HAVE_ZSTD_SUPPORT),
>  	FEATURE_SUPPORT("libpfm4", HAVE_LIBPFM),
>  	FEATURE_SUPPORT("libtraceevent", HAVE_LIBTRACEEVENT),
> -	FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL);
> +	FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL),
>  
>  	/* this should remain at end, to know the array end */
>  	FEATURE_SUPPORT(NULL, _)

-- 

- Arnaldo