Re: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Leo Yan <leo.yan@arm.com>]

On Tue, May 27, 2025 at 03:30:06PM -0400, Liang, Kan wrote:

[...]

> 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 other cases
> that want to avoid the stop. So the latter is implemented as below.

Yes.  I agreed the fix below is more general and confirmed it can fix
the observed issue.

> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 947ad12dfdbe..66f02f46595c 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -303,6 +303,7 @@ struct perf_event_pmu_context;
>  #define PERF_PMU_CAP_AUX_OUTPUT			0x0080
>  #define PERF_PMU_CAP_EXTENDED_HW_TYPE		0x0100
>  #define PERF_PMU_CAP_AUX_PAUSE			0x0200
> +#define PERF_PMU_CAP_NO_THROTTLE_STOP		0x0400
> 
>  /**
>   * pmu::scope
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 8327ab0ee641..596597886d96 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2655,7 +2655,8 @@ 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);
> +	if (!(event->pmu->capabilities & PERF_PMU_CAP_NO_THROTTLE_STOP))
> +		event->pmu->stop(event, 0);

A background info is that even a PMU event is not stopped when
throttling, we still need to re-enable it.  This is why we don't do
particualy handling for PERF_PMU_CAP_NO_THROTTLE_STOP in
perf_event_unthrottle().

Maybe it is deserved add a comment for easier understanding.

Thanks,
Leo

>  	event->hw.interrupts = MAX_INTERRUPTS;
>  	perf_log_throttle(event, 0);
>  }
> @@ -11846,7 +11847,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,
> @@ -11928,7 +11930,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,
> 
> 
> Thanks,
> Kan
> 
>