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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y2vz3zmn/jIFkCKp@kernel.org>
Date:   Wed, 9 Nov 2022 15:39:27 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Jiri Olsa <jolsa@...nel.org>, Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Ian Rogers <irogers@...gle.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        linux-perf-users@...r.kernel.org, Leo Yan <leo.yan@...aro.org>,
        German Gomez <german.gomez@....com>,
        Zhengjun Xing <zhengjun.xing@...ux.intel.com>,
        James Clark <james.clark@....com>
Subject: Re: [PATCH 01/12] perf test: Add -w/--workload option

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@...lbox perf]$ git show 01513fdc18f395db
commit 01513fdc18f395dbcc924bc5e9962b12f86f947a
Author: Ian Rogers <irogers@...gle.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@...gle.com>
    Cc: Adrian Hunter <adrian.hunter@...el.com>
    Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
    Cc: Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>
    Cc: German Gomez <german.gomez@....com>
    Cc: Ingo Molnar <mingo@...hat.com>
    Cc: Jiri Olsa <jolsa@...nel.org>
    Cc: Leo Yan <leo.yan@...aro.org>
    Cc: Mark Rutland <mark.rutland@....com>
    Cc: Namhyung Kim <namhyung@...nel.org>
    Cc: Peter Zijlstra <peterz@...radead.org>
    Cc: Stephane Eranian <eranian@...gle.com>
    Link: https://lore.kernel.org/r/20221024181913.630986-7-irogers@google.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.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@...lbox 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