All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Swapnil Sapkal <swapnil.sapkal@amd.com>
Cc: peterz@infradead.org, mingo@redhat.com, namhyung@kernel.org,
	irogers@google.com, james.clark@linaro.org, mark.rutland@arm.com,
	alexander.shishkin@linux.intel.com, jolsa@kernel.org,
	adrian.hunter@intel.com, ravi.bangoria@amd.com,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 0/3] perf: Fix SIGCHLD vs pause() race with short-lived workloads
Date: Wed, 20 May 2026 16:58:17 -0300	[thread overview]
Message-ID: <ag4SWXqjAWsjuCsE@x1> (raw)
In-Reply-To: <20260520102017.293419-1-swapnil.sapkal@amd.com>

On Wed, May 20, 2026 at 10:20:14AM +0000, Swapnil Sapkal wrote:
> Several perf subcommands (sched stats, lock contention) use the pattern
> of forking a workload child, calling evlist__start_workload() to uncork
> it, and then calling pause() to wait for a signal (typically SIGCHLD
> when the child exits, or SIGINT/SIGTERM from the user).
> 
> This pattern has a race condition: if the workload is very short-lived,
> the child can exit and deliver SIGCHLD in the window between
> evlist__start_workload() and pause(). Since pause() only returns when a
> signal is received *while the process is suspended*, and SIGCHLD has
> already been delivered and handled by the empty sighandler(), pause()
> blocks indefinitely.
> 
> The fix replaces pause() with a simpler approach:
> 
>  - The signal handler now sets a 'volatile sig_atomic_t done' flag
>    to record delivery.  The flag is reset before handler registration
>    so that a signal arriving during the setup phase is not discarded.
> 
>  - Replace pause() with a unified loop:
> 
>        while (!done) {
>            if (argc && waitpid(evlist->workload.pid, NULL, WNOHANG) > 0)
>                break;
>            sleep(1);
>        }
> 
>    This handles both workload mode (child exit detected via waitpid)
>    and system-wide mode (user sends SIGINT/SIGTERM setting done).
>    Using WNOHANG avoids the SA_RESTART problem where a blocking
>    waitpid() would auto-restart and ignore the done flag if the child
>    doesn't exit on signal.
> 
> Three call sites are affected across two files:
>   - perf_sched__schedstat_record() in builtin-sched.c
>   - perf_sched__schedstat_live()   in builtin-sched.c
>   - __cmd_contention()             in builtin-lock.c
> 
> The two pause() sites in builtin-kwork.c are NOT affected because they
> do not register SIGCHLD or fork workload children; they only wait for
> user-initiated SIGINT/SIGTERM.

Thanks, applied to perf-tools-next, for v7.2.

- Arnaldo

      parent reply	other threads:[~2026-05-20 19:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-20 10:20 [PATCH v4 0/3] perf: Fix SIGCHLD vs pause() race with short-lived workloads Swapnil Sapkal
2026-05-20 10:20 ` [PATCH v4 1/3] perf sched stats: Fix SIGCHLD vs pause() race in schedstat_record() Swapnil Sapkal
2026-05-20 10:20 ` [PATCH v4 2/3] perf sched stats: Fix SIGCHLD vs pause() race in schedstat_live() Swapnil Sapkal
2026-05-20 11:18   ` sashiko-bot
2026-05-20 10:20 ` [PATCH v4 3/3] perf lock contention: Fix SIGCHLD vs pause() race in __cmd_contention() Swapnil Sapkal
2026-05-20 19:58 ` Arnaldo Carvalho de Melo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ag4SWXqjAWsjuCsE@x1 \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=ravi.bangoria@amd.com \
    --cc=swapnil.sapkal@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.