Re: [PATCH 4/4] stm class: ftrace: use different channel accroding to CPU

[Steven Rostedt]

On Mon, 20 Jul 2020 10:21:17 +0800
Tingwei Zhang <tingwei@xxxxxxxxxxxxxx> wrote:

> To avoid mixup of packets from differnt ftrace packets simultaneously,
> use different channel for packets from different CPU.
> 
> Signed-off-by: Tingwei Zhang <tingwei@xxxxxxxxxxxxxx>
> ---
>  drivers/hwtracing/stm/ftrace.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
> index ce868e095410..6c3bce11c8d3 100644
> --- a/drivers/hwtracing/stm/ftrace.c
> +++ b/drivers/hwtracing/stm/ftrace.c
> @@ -10,7 +10,7 @@
>  #include <linux/stm.h>
>  #include <linux/trace.h>
>  
> -#define STM_FTRACE_NR_CHANNELS 1
> +#define STM_FTRACE_NR_CHANNELS NR_CPUS

I would make this dynamic. If this causes any kind of allocation, you
do not want NR_CPUS. That's hardcoded by the distribution and is the
max number of CPUs. With machines with hundreds of CPUs today, that
could easily be 256 or 512 or more.

Instead, in the stm_ftrace_init() I would just assign it to
nr_online_cpus().

	stm_ftrace.data.nr_chans = nr_online_cpus();

Or nr_possible_cpus().

-- Steve


>  #define STM_FTRACE_CHAN 0
>  
>  static int stm_ftrace_link(struct stm_source_data *data);
> @@ -37,8 +37,9 @@ static void notrace
>  stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
>  {
>  	struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
> +	unsigned int cpu = smp_processor_id();
>  
> -	stm_source_write(&stm->data, STM_FTRACE_CHAN, buf, len);
> +	stm_source_write(&stm->data, STM_FTRACE_CHAN + cpu, buf, len);
>  }
>  
>  static int stm_ftrace_link(struct stm_source_data *data)