Re: [PATCH] futex: Remove the owner check when waking task inhandle_futex_death

[Darren Hart]

On Thu, 2013-09-26 at 09:09 +0800, zhang.yi20@xxxxxxxxxx wrote:
> Hi all,
> 
> Task processes all its owned robust futex when it is exiting,
> to ensure the futexes can be taken by other tasks.
> 
> Though this can not work good in sometimes.
> Think about this sceneï
> 1. A robust mutex is shared for two processes, each process has
>    multi threads to lock the mutex.
> 2. One of the threads locks the mutex, and the others are waiting
>    and sorted in order of priority.
> 3. The process to which the mutex owner thread belongs is dying
>    without unlocking the mutexïand handle_futex_death is invoked
>    to wake the first waiter.
> 4. If the first waiter belongs to the same processïit has no chance
>    to return to the userspace to lock the mutex, and it won't wake
>    the next waiter because it is not the owner of the mutex.
> 5. The rest waiters of the other process may block forever.
> 
> This patch remove the owner check when waking task in handle_futex_death.
> If above occured, The dying task can wake the next waiter by processing its list_op_pending.
> The waked task could return to userspace and try to lock the mutex again.
> 

The problem is if you allow the non-owner to do the wake, you risk
multiple threads calling futex_wake(). Or is that your intention? Wake
one waiter for every thread that calls handle_futex_death()?

> 
> Signed-off-by: Zhang Yi <zhang.yi20@xxxxxxxxxx>
> Reviewed-by: Xie Baoyou <xie.baoyou@xxxxxxxxxx>
> Reviewed-by: Lu Zhongjun <lu.zhongjun@xxxxxxxxxx>
> 
> 
> 
> --- linux/kernel/futex.c	2013-09-25 09:24:34.639634244 +0000
> +++ linux/kernel/futex.c	2013-09-25 10:12:17.619673546 +0000
> @@ -2541,14 +2541,15 @@ retry:
>  		}
>  		if (nval != uval)
>  			goto retry;
> -
> -		/*
> -		 * Wake robust non-PI futexes here. The wakeup of
> -		 * PI futexes happens in exit_pi_state():
> -		 */
> -		if (!pi && (uval & FUTEX_WAITERS))
> -			futex_wake(uaddr, 1, 1, FUTEX_BITSET_MATCH_ANY);
>  	}
> +
> +	/*
> +	 * Wake robust non-PI futexes here. The wakeup of
> +	 * PI futexes happens in exit_pi_state():
> +	 */
> +	if (!pi)


Why did you drop the FUTEX_WAITERS condition?

You sent a different patch earlier this year that didn't appear to get
any review:

https://lkml.org/lkml/2013/4/8/65

This one woke all the waiters and let them sort it out.

It seems we've hashed through this already, but I'm not finding the
email logs and I don't recall off the top of my head.

> +		futex_wake(uaddr, 1, 1, FUTEX_BITSET_MATCH_ANY);
> +
>  	return 0;
>  }

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/