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

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

...

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