Re: [PATCH 01/12] perf test: Add -w/--workload option

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

Em Wed, Nov 09, 2022 at 09:46:24AM -0800, Namhyung Kim escreveu:
> --- /dev/null
> +++ b/tools/perf/tests/workloads/noploop.c
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <unistd.h>
> +#include <linux/compiler.h>
> +#include "../tests.h"
> +
> +static volatile int done;
> +
> +static void sighandler(int sig __maybe_unused)
> +{
> +	done = 1;
> +}

You forgot to do what was done in:

92ea0720ba9cf7f0 perf trace: Use sig_atomic_t to avoid undefined behaviour in a signal handler
691768968f2a13eb perf top: Use sig_atomic_t to avoid undefined behaviour in a signal handler
01513fdc18f395db perf stat: Use sig_atomic_t to avoid undefined behaviour in a signal handler
057929f9d083e80c perf session: Change type to avoid undefined behaviour in a signal handler
853596fb71f7c2f7 perf ftrace: Use sig_atomic_t to avoid UB
7f3374299f9762ba perf daemon: Use sig_atomic_t to avoid UB
8ed28c2b56b78442 perf record: Use sig_atomic_t for signal handlers
f3c9bd4e16a503cb perf build: Update to C standard to gnu11

To speed up the process here is one of those csets:

⬢[acme@toolbox perf]$ git show 01513fdc18f395db
commit 01513fdc18f395dbcc924bc5e9962b12f86f947a
Author: Ian Rogers <irogers@google.com>
Date:   Mon Oct 24 11:19:11 2022 -0700

    perf stat: Use sig_atomic_t to avoid undefined behaviour in a signal handler

    Use sig_atomic_t for variables written/accessed in signal
    handlers. This is undefined behavior as per:

      https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

    Signed-off-by: Ian Rogers <irogers@google.com>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
    Cc: German Gomez <german.gomez@arm.com>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Leo Yan <leo.yan@linaro.org>
    Cc: Mark Rutland <mark.rutland@arm.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Stephane Eranian <eranian@google.com>
    Link: https://lore.kernel.org/r/20221024181913.630986-7-irogers@google.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index e52601a54b26d669..d5e1670bca204450 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -173,7 +173,7 @@ static struct target target = {

 #define METRIC_ONLY_LEN 20

-static volatile pid_t          child_pid                       = -1;
+static volatile sig_atomic_t   child_pid                       = -1;
 static int                     detailed_run                    =  0;
 static bool                    transaction_run;
 static bool                    topdown_run                     = false;
@@ -208,7 +208,7 @@ struct perf_stat {
 static struct perf_stat                perf_stat;
 #define STAT_RECORD            perf_stat.record

-static volatile int done = 0;
+static volatile sig_atomic_t done = 0;

 static struct perf_stat_config stat_config = {
        .aggr_mode              = AGGR_GLOBAL,
@@ -580,7 +580,7 @@ static void disable_counters(void)
        }
 }

-static volatile int workload_exec_errno;
+static volatile sig_atomic_t workload_exec_errno;

 /*
  * evlist__prepare_workload will send a SIGUSR1
@@ -1039,7 +1039,7 @@ static void print_counters(struct timespec *ts, int argc, const char **argv)
        evlist__print_counters(evsel_list, &stat_config, &target, ts, argc, argv);
 }

-static volatile int signr = -1;
+static volatile sig_atomic_t signr = -1;

 static void skip_signal(int signo)
 {
⬢[acme@toolbox perf]$

> +
> +static int noploop(int argc, const char **argv)
> +{
> +	int sec = 1;
> +
> +	if (argc > 0)
> +		sec = atoi(argv[0]);
> +
> +	signal(SIGINT, sighandler);
> +	signal(SIGALRM, sighandler);
> +	alarm(sec);
> +
> +	while (!done)
> +		continue;
> +
> +	return 0;
> +}
> +
> +DEFINE_WORKLOAD(noploop);
> -- 
> 2.38.1.431.g37b22c650d-goog

-- 

- Arnaldo