[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8e8f51d8-af64-4d9e-934b-c0ee9f131293@linux.ibm.com>
Date: Tue, 3 Jun 2025 18:15:16 +0530
From: Venkat Rao Bagalkote <venkat88@...ux.ibm.com>
To: kan.liang@...ux.intel.com, peterz@...radead.org, mingo@...hat.com,
namhyung@...nel.org, irogers@...gle.com, mark.rutland@....com,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
Athira Rajeev <atrajeev@...ux.ibm.com>,
Madhavan Srinivasan <maddy@...ux.ibm.com>
Cc: eranian@...gle.com, ctshao@...gle.com, tmricht@...ux.ibm.com,
leo.yan@....com, Aishwarya TCV <aishwarya.tcv@....com>
Subject: Re: [PATCH V2] perf: Fix the throttle error of some clock events
On 28/05/25 11:28 pm, kan.liang@...ux.intel.com wrote:
> From: Kan Liang <kan.liang@...ux.intel.com>
>
> The Arm CI reports RCU stall, which can be reproduced by the below perf
> command.
> perf record -a -e cpu-clock -- sleep 2
This issue is reported on IBM CI as well. But the issue is hit, while
running perf test (specifically while runningĀ 76: perf list tests)
>
> The cpu-clock and task_clock are two special SW events, which rely on
> the hrtimer. Instead of invoking the stop(), the HRTIMER_NORESTART is
> returned to stop the timer. Because the hrtimer interrupt handler cannot
> cancel itself, which causes infinite loop.
>
> There may be two ways to fix it.
> - Add a check of MAX_INTERRUPTS in the event_stop. Return immediately if
> the stop is invoked by the throttle.
> - Introduce a PMU flag to track the case. Avoid the event_stop in
> perf_event_throttle() if the flag is detected.
>
> The latter looks more generic. It may be used if there are more other
> cases that want to avoid the stop later. The latter is implemented.
Tested this patch by applying on the linux-mainline kernel on IBM Power9
system, and it fixes the issue. Hence,
Tested-by: Venkat Rao Bagalkote <venkat88@...ux.ibm.com>
Regards,
Venkat.
>
> Reported-by: Leo Yan <leo.yan@....com>
> Reported-by: Aishwarya TCV <aishwarya.tcv@....com>
> Closes: https://lore.kernel.org/lkml/20250527161656.GJ2566836@e132581.arm.com/
> Tested-by: Leo Yan <leo.yan@....com>
> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
> ---
>
> Changes since V1:
> - Rebase on top of the latest tip.git
> - Add Tested-by from Leo
>
> include/linux/perf_event.h | 1 +
> kernel/events/core.c | 23 ++++++++++++++++++++---
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 52dc7cfab0e0..97a747a97a50 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -305,6 +305,7 @@ struct perf_event_pmu_context;
> #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0100
> #define PERF_PMU_CAP_AUX_PAUSE 0x0200
> #define PERF_PMU_CAP_AUX_PREFER_LARGE 0x0400
> +#define PERF_PMU_CAP_NO_THROTTLE_STOP 0x0800
>
> /**
> * pmu::scope
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f34c99f8ce8f..abd19bb571e3 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2656,7 +2656,22 @@ static void perf_event_unthrottle(struct perf_event *event, bool start)
>
> static void perf_event_throttle(struct perf_event *event)
> {
> - event->pmu->stop(event, 0);
> + /*
> + * Some PMUs, e.g., cpu-clock and task_clock, may rely on
> + * a special mechanism (hrtimer) to manipulate counters.
> + * The regular stop doesn't work, since the hrtimer interrupt
> + * handler cannot cancel itself.
> + *
> + * The stop should be avoided for such cases. Let the
> + * driver-specific code handle it.
> + *
> + * The counters will eventually be disabled in the driver-specific
> + * code. In unthrottle, they still need to be re-enabled.
> + * There is no handling for PERF_PMU_CAP_NO_THROTTLE_STOP in
> + * the perf_event_unthrottle().
> + */
> + if (!(event->pmu->capabilities & PERF_PMU_CAP_NO_THROTTLE_STOP))
> + event->pmu->stop(event, 0);
> event->hw.interrupts = MAX_INTERRUPTS;
> if (event == event->group_leader)
> perf_log_throttle(event, 0);
> @@ -11848,7 +11863,8 @@ static int cpu_clock_event_init(struct perf_event *event)
> static struct pmu perf_cpu_clock = {
> .task_ctx_nr = perf_sw_context,
>
> - .capabilities = PERF_PMU_CAP_NO_NMI,
> + .capabilities = PERF_PMU_CAP_NO_NMI |
> + PERF_PMU_CAP_NO_THROTTLE_STOP,
> .dev = PMU_NULL_DEV,
>
> .event_init = cpu_clock_event_init,
> @@ -11930,7 +11946,8 @@ static int task_clock_event_init(struct perf_event *event)
> static struct pmu perf_task_clock = {
> .task_ctx_nr = perf_sw_context,
>
> - .capabilities = PERF_PMU_CAP_NO_NMI,
> + .capabilities = PERF_PMU_CAP_NO_NMI |
> + PERF_PMU_CAP_NO_THROTTLE_STOP,
> .dev = PMU_NULL_DEV,
>
> .event_init = task_clock_event_init,
Powered by blists - more mailing lists