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

From: Arnaldo Carvalho de Melo
Date: Wed Nov 09 2022 - 13:39:38 EST


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@xxxxxxxxxx>
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@xxxxxxxxxx>
Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
Cc: Alexey Bayduraev <alexey.v.bayduraev@xxxxxxxxxxxxxxx>
Cc: German Gomez <german.gomez@xxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Cc: Leo Yan <leo.yan@xxxxxxxxxx>
Cc: Mark Rutland <mark.rutland@xxxxxxx>
Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Stephane Eranian <eranian@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20221024181913.630986-7-irogers@xxxxxxxxxx
Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>

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