Re: [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary

[Liang, Kan]

On 2025-01-22 2:41 a.m., lirongqing wrote:
> From: Li RongQing <lirongqing@xxxxxxxxx>
> 
> Avoid unnecessary per-CPU memory allocation on unsupported CPUs,
> this can save 12K memory for each CPU
> 
> Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx>

Acked-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>

Thanks,
Kan

> ---
>  arch/x86/events/intel/bts.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
> index 8f78b0c..8e09319 100644
> --- a/arch/x86/events/intel/bts.c
> +++ b/arch/x86/events/intel/bts.c
> @@ -36,7 +36,7 @@ enum {
>  	BTS_STATE_ACTIVE,
>  };
>  
> -static DEFINE_PER_CPU(struct bts_ctx, bts_ctx);
> +static struct bts_ctx __percpu *bts_ctx;
>  
>  #define BTS_RECORD_SIZE		24
>  #define BTS_SAFETY_MARGIN	4080
> @@ -231,7 +231,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle);
>  
>  static void __bts_event_start(struct perf_event *event)
>  {
> -	struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> +	struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>  	struct bts_buffer *buf = perf_get_aux(&bts->handle);
>  	u64 config = 0;
>  
> @@ -260,7 +260,7 @@ static void __bts_event_start(struct perf_event *event)
>  static void bts_event_start(struct perf_event *event, int flags)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> -	struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> +	struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>  	struct bts_buffer *buf;
>  
>  	buf = perf_aux_output_begin(&bts->handle, event);
> @@ -290,7 +290,7 @@ static void bts_event_start(struct perf_event *event, int flags)
>  
>  static void __bts_event_stop(struct perf_event *event, int state)
>  {
> -	struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> +	struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>  
>  	/* ACTIVE -> INACTIVE(PMI)/STOPPED(->stop()) */
>  	WRITE_ONCE(bts->state, state);
> @@ -305,7 +305,7 @@ static void __bts_event_stop(struct perf_event *event, int state)
>  static void bts_event_stop(struct perf_event *event, int flags)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> -	struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> +	struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>  	struct bts_buffer *buf = NULL;
>  	int state = READ_ONCE(bts->state);
>  
> @@ -338,7 +338,7 @@ static void bts_event_stop(struct perf_event *event, int flags)
>  
>  void intel_bts_enable_local(void)
>  {
> -	struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> +	struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>  	int state = READ_ONCE(bts->state);
>  
>  	/*
> @@ -358,7 +358,7 @@ void intel_bts_enable_local(void)
>  
>  void intel_bts_disable_local(void)
>  {
> -	struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> +	struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>  
>  	/*
>  	 * Here we transition from ACTIVE to INACTIVE;
> @@ -450,7 +450,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle)
>  int intel_bts_interrupt(void)
>  {
>  	struct debug_store *ds = this_cpu_ptr(&cpu_hw_events)->ds;
> -	struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> +	struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>  	struct perf_event *event = bts->handle.event;
>  	struct bts_buffer *buf;
>  	s64 old_head;
> @@ -518,7 +518,7 @@ static void bts_event_del(struct perf_event *event, int mode)
>  
>  static int bts_event_add(struct perf_event *event, int mode)
>  {
> -	struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> +	struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>  	struct hw_perf_event *hwc = &event->hw;
>  
> @@ -605,6 +605,10 @@ static __init int bts_init(void)
>  		return -ENODEV;
>  	}
>  
> +	bts_ctx = alloc_percpu(struct bts_ctx);
> +	if (!bts_ctx)
> +		return -ENOMEM;
> +
>  	bts_pmu.capabilities	= PERF_PMU_CAP_AUX_NO_SG | PERF_PMU_CAP_ITRACE |
>  				  PERF_PMU_CAP_EXCLUSIVE;
>  	bts_pmu.task_ctx_nr	= perf_sw_context;