Re: [PATCH] Avoid unnecessary busy loop in __lll_timedlock_wait on ARM.

["Joseph S. Myers" <joseph@codesourcery.com>]

On Thu, 31 Jan 2013, katsuki.uwatoko@toshiba.co.jp wrote:

> Hi,
> 
> I have found an issue in __lll_timedlock_wait on ARM.

I think the busy loop should have a bug filed in Bugzilla, as a 
user-visible bug - could you file that bug?

> The following sequence causes unnecessary busy loop. 
> 
> "A thread" gets the lock. (futex = 1)
> "B thread" tries to get the lock, and has not called futex syscall yet. (futex = 2)
> "A thread" releases the lock (futex = 0)
> "C thread" gets the lock, and does not call futex syscall because of futex=0 (futex = 1)
> "B thread" calls futex syscall, and returns with an error.
>            Because futex syscall in Linux Kernel checks if the val is changed 
>            from 2, which is the 3rd arg of the syscall at futex_wait_setup(),
>            but in this case futex is 1.
> "B thread" tries to get the lock in userspace but cannot get it 
>            because futex is not 0.
>            After all the thread keeps calling futex syscall 
>            until "C thread" will release it (futex = 0) or timeout.
> 
> Therefore I think that the value should be set 2 in every loop 
> the same as __lll_lock_wait_private, and attached a patch for this issue.

Carlos, any comments on this patch 
<http://sourceware.org/ml/libc-ports/2013-01/msg00084.html>?  It makes the 
ARM version of __lll_timedlock_wait closer to the HPPA version, but I 
don't know if many of the differences between different architecture 
versions of this code are really deliberate....

Would you agree that the generic Linux version 
(nptl/sysdeps/unix/sysv/linux/lowlevellock.c) doesn't need such a change 
because the loop is using atomic_exchange_acq rather than 
atomic_compare_and_exchange_bool_acq, so is already setting the futex to 2 
in every loop iteration?

-- 
Joseph S. Myers
joseph@codesourcery.com