On Sat, Jul 10, 2021 at 03:34:24PM +0300, Adrian Hunter wrote:

On 4/07/21 10:16 am, Leo Yan wrote:

quoted

The main purpose for using __sync built-in functions is to support
compat mode for 32-bit perf with 64-bit kernel.  But using these
built-in functions might cause couple potential issues.

Firstly, __sync functions originally support Intel Itanium processoer [1]
but it cannot promise to support all 32-bit archs.  Now these
functions have become the legacy functions.

As Peter also pointed out the logic issue in the function
auxtrace_mmap__write_tail(), it does a cmpxchg with 0 values to load
old_tail, and then executes a further cmpxchg with old_tail to write
the new tail.  If consider the aux_tail might be assigned to '0' in the
middle of loops, this can introduce mess for AUX buffer if the kernel
fetches the temporary value '0'.

That is not exactly true. The definition of __sync_*_compare_and_swap is
"if the current value of *ptr is oldval, then write newval into *pt"
so replacing zero with zero won't make any difference, but it will return
the old value in any case.  Probably better to leave out that paragraph.

Okay, I admit the paragraph is not right, will drop it to avoid
confusion.  Thanks for review!

Leo