Re: [patch 1/3] ipc/sem: fix -rt livelock

[Manfred Spraul <manfred@colorfullife.com>]

Hi Mike,

On 09/13/2013 08:12 AM, Mike Galbraith wrote:
> goto again loop can and does induce livelock in -rt.  Remove it.
>
> spin_unlock_wait(lock) in -rt kernels takes/releases the lock in question, so
> all it takes to create a self perpetuating loop is for one task to start the
> ball rolling by taking the array lock, other tasks see this, and react by
> take/release/retry endlessly.
>
> Signed-off-by: Mike Galbraith <bitbucket@online.de>
>
> ---
>   ipc/sem.c |   56 ++++++++++++++++++++++++++++++++++++++------------------
>   1 file changed, 38 insertions(+), 18 deletions(-)
>
> Index: linux-2.6/ipc/sem.c
> ===================================================================
> --- linux-2.6.orig/ipc/sem.c
> +++ linux-2.6/ipc/sem.c
> @@ -247,11 +256,22 @@ static inline int sem_lock(struct sem_ar
>   		 */
>    lock_array:
>   		spin_lock(&sma->sem_perm.lock);
> + wait_array:
>   		for (i = 0; i < sma->sem_nsems; i++) {
> -			struct sem *sem = sma->sem_base + i;
> +			sem = sma->sem_base + i;
> +#ifdef CONFIG_PREEMPT_RT_BASE
> +			if (spin_is_locked(&sem->lock))
> +#endif
>   			spin_unlock_wait(&sem->lock);
>   		}
>   
I don't like this part of the change:
It reads like a micro-optimization for spin_unlock_wait() within the 
ipc/sem.c code.

If spin_unlock_wait() for CONFIG_PREEMPT_RT_BASE is broken, then the 
implementation of spin_unlock_wait() should be fixed.

Something like
#define spin_unlock_wait(x) if(spin_is_locked(x)) __spin_unlock_wait(x)

--
     Manfred