Re: [PATCH v1 1/2] perf auxtrace: Change to use SMP memory barriers

[Adrian Hunter <adrian.hunter@intel.com>]

On 19/05/21 5:03 pm, Leo Yan wrote:
> The AUX ring buffer's head and tail can be accessed from multiple CPUs
> on SMP system, so changes to use SMP memory barriers to replace the
> uniprocessor barriers.

I don't think user space should attempt to be SMP-aware.

For perf tools, on __x86_64__ it looks like smp_rmb() is only a compiler barrier, whereas
rmb() is a "lfence" memory barrier instruction, so this patch does not
seem to do what the commit message says at least for x86.

With regard to the AUX area, we don't know in general how data gets there,
so using memory barriers seems sensible.

> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/util/auxtrace.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index 472c0973b1f1..8bed284ccc82 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -452,7 +452,7 @@ static inline u64 auxtrace_mmap__read_snapshot_head(struct auxtrace_mmap *mm)
>  	u64 head = READ_ONCE(pc->aux_head);
>  
>  	/* Ensure all reads are done after we read the head */
> -	rmb();
> +	smp_rmb();
>  	return head;
>  }
>  
> @@ -466,7 +466,7 @@ static inline u64 auxtrace_mmap__read_head(struct auxtrace_mmap *mm)
>  #endif
>  
>  	/* Ensure all reads are done after we read the head */
> -	rmb();
> +	smp_rmb();
>  	return head;
>  }
>  
> @@ -478,7 +478,7 @@ static inline void auxtrace_mmap__write_tail(struct auxtrace_mmap *mm, u64 tail)
>  #endif
>  
>  	/* Ensure all reads are done before we write the tail out */
> -	mb();
> +	smp_mb();
>  #if BITS_PER_LONG == 64 || !defined(HAVE_SYNC_COMPARE_AND_SWAP_SUPPORT)
>  	pc->aux_tail = tail;
>  #else
>