Re: [RFC/PATCH 1/2] perf check: Add 'system' subcommand

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

On Thu, Sep 18, 2025 at 08:48:10AM -0700, Ian Rogers wrote:
> On Wed, Sep 17, 2025 at 11:39 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > +/**
> > + * Usage: 'perf check system <settings>'
> > + *
> > + * Show system settings that affect perf behavior.
> > + */
> > +static int subcommand_system(int argc, const char **argv)
> > +{
> > +#define PERF_SYSCTL(name) { #name, system_help_##name }
> > +       struct {
> > +               const char *name;
> > +               const char *(*help)(int value);
> > +       } sysctls[] = {
> > +               PERF_SYSCTL(perf_event_paranoid),
> > +               PERF_SYSCTL(perf_event_max_stack),
> > +               PERF_SYSCTL(perf_event_mlock_kb),
> > +               PERF_SYSCTL(nmi_watchdog),
> > +               PERF_SYSCTL(kptr_restrict),
> > +       };
> > +#undef PERF_SYSCTL
> > +
> > +       argc = parse_options(argc, argv, check_system_options,
> > +                            check_system_usage, 0);
> > +
> > +       for (size_t i = 0; i < ARRAY_SIZE(sysctls); i++) {
> > +               int value;
> > +
> > +               if (argc) {
> > +                       bool found = false;
> > +
> > +                       /* only show entries match to command line arguments */
> > +                       for (int k = 0; k < argc; k++) {
> > +                               if (strstr(sysctls[i].name, argv[k])) {
> > +                                       found = true;
> > +                                       break;
> > +                               }
> > +                       }
> > +                       if (!found)
> > +                               continue;
> > +               }
> > +
> > +               value = read_sysctl_kernel_int(sysctls[i].name);
> > +               printf("%-20s = %d", sysctls[i].name, value);
> > +               if (!quiet)
> > +                       printf("\t# %s", sysctls[i].help(value));
> > +               printf("\n");
> > +       }
> > +
> > +       return 0;
> 
> This looks useful! Rather than returning 0 should this return
> something indicating whether perf does or doesn't have the permission?
> In that case, what about root? We have this pattern in our shell
> tests:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/lib/stat_output.sh?h=perf-tools-next
> ```
> function ParanoidAndNotRoot()
> {
> [ "$(id -u)" != 0 ] && [ "$(cat /proc/sys/kernel/perf_event_paranoid)" -gt $1 ]
> }
> ```
> I wonder things like this can use these values instead.

Yeah, I thought about that as well, to use these checks in the perf test
shell entries.

- Arnaldo