From: Peter Zijlstra <peterz@infradead.org>
To: Frederic Weisbecker <frederic@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>,
LKML <linux-kernel@vger.kernel.org>,
"Liang, Kan" <kan.liang@linux.intel.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Ian Rogers <irogers@google.com>, Jiri Olsa <jolsa@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Namhyung Kim <namhyung@kernel.org>,
Ravi Bangoria <ravi.bangoria@amd.com>,
linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 3/4] perf: Remove too early and redundant CPU hotplug handling
Date: Thu, 24 Apr 2025 18:32:21 +0200 [thread overview]
Message-ID: <20250424163221.GD18306@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20250424161128.29176-4-frederic@kernel.org>
On Thu, Apr 24, 2025 at 06:11:27PM +0200, Frederic Weisbecker wrote:
> The CPU hotplug handlers are called twice: at prepare and online stage.
>
> Their role is to:
>
> 1) Enable/disable a CPU context. This is irrelevant and even buggy at
> the prepare stage because the CPU is still offline. On early
> secondary CPU up, creating an event attached to that CPU might
> silently fail because the CPU context is observed as online but the
> context installation's IPI failure is ignored.
>
> 2) Update the scope cpumasks and re-migrate the events accordingly in
> the CPU down case. This is irrelevant at the prepare stage.
>
> 3) Remove the events attached to the context of the offlining CPU. It
> even uses an (unnecessary) IPI for it. This is also irrelevant at the
> prepare stage.
>
> Also none of the *_PREPARE and *_STARTING architecture perf related CPU
> hotplug callbacks rely on CPUHP_PERF_PREPARE.
>
> CPUHP_AP_PERF_ONLINE is enough and the right place to perform the work.
Oh hey, that's curious indeed.
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
> include/linux/cpuhotplug.h | 1 -
> kernel/cpu.c | 5 -----
> 2 files changed, 6 deletions(-)
>
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 1987400000b4..df366ee15456 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -60,7 +60,6 @@ enum cpuhp_state {
> /* PREPARE section invoked on a control CPU */
> CPUHP_OFFLINE = 0,
> CPUHP_CREATE_THREADS,
> - CPUHP_PERF_PREPARE,
> CPUHP_PERF_X86_PREPARE,
> CPUHP_PERF_X86_AMD_UNCORE_PREP,
> CPUHP_PERF_POWER,
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index b08bb34b1718..a59e009e0be4 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -2069,11 +2069,6 @@ static struct cpuhp_step cpuhp_hp_states[] = {
> .teardown.single = NULL,
> .cant_stop = true,
> },
> - [CPUHP_PERF_PREPARE] = {
> - .name = "perf:prepare",
> - .startup.single = perf_event_init_cpu,
> - .teardown.single = perf_event_exit_cpu,
> - },
> [CPUHP_RANDOM_PREPARE] = {
> .name = "random:prepare",
> .startup.single = random_prepare_cpu,
> --
> 2.48.1
>
next prev parent reply other threads:[~2025-04-24 16:32 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-24 16:11 [PATCH 0/4] perf fixes Frederic Weisbecker
2025-04-24 16:11 ` [PATCH 1/4] perf: Fix failing inherit_event() doing extra refcount decrement on parent Frederic Weisbecker
2025-04-24 16:16 ` Peter Zijlstra
2025-05-08 10:34 ` [tip: perf/core] " tip-bot2 for Frederic Weisbecker
2025-05-08 19:55 ` tip-bot2 for Frederic Weisbecker
2025-04-24 16:11 ` [PATCH 2/4] perf: Fix irq work dereferencing garbage Frederic Weisbecker
2025-04-24 16:30 ` Peter Zijlstra
2025-04-28 11:11 ` Frederic Weisbecker
2025-05-02 10:29 ` Peter Zijlstra
2025-05-02 11:30 ` Peter Zijlstra
2025-05-02 12:04 ` Frederic Weisbecker
2025-05-02 11:58 ` Frederic Weisbecker
2025-04-24 16:11 ` [PATCH 3/4] perf: Remove too early and redundant CPU hotplug handling Frederic Weisbecker
2025-04-24 16:32 ` Peter Zijlstra [this message]
2025-05-08 10:34 ` [tip: perf/core] " tip-bot2 for Frederic Weisbecker
2025-05-08 19:55 ` tip-bot2 for Frederic Weisbecker
2025-04-24 16:11 ` [PATCH 4/4] perf: Fix confusing aux iteration Frederic Weisbecker
2025-05-08 10:34 ` [tip: perf/core] " tip-bot2 for Frederic Weisbecker
2025-05-08 19:55 ` tip-bot2 for Frederic Weisbecker
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=20250424163221.GD18306@noisy.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=frederic@kernel.org \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--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=ravi.bangoria@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox