Messages in this thread | | | Date | Sun, 11 Jul 2021 15:41:16 +0800 | From | Leo Yan <> | Subject | Re: [PATCH v3 05/10] perf auxtrace: Drop legacy __sync functions |
| |
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
| |