Re: [PATCH 2/5] futex: Check for pi futex_q only once

Darren Hart
2013-11-23T06:33:52Z

On Fri, 2013-11-22 at 16:56 -0800, Davidlohr Bueso wrote:
> All wake_futex() callers already verify that the we are not dealing with
> a pi futex_q, so we can remove the redundant WARN() check, as this is never
> triggered anyway.
>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Darren Hart <dvhart@linux.intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Jeff Mahoney <jeffm@suse.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Scott Norton <scott.norton@hp.com>
> Cc: Tom Vaden <tom.vaden@hp.com>
> Cc: Aswin Chandramouleeswaran <aswin@hp.com>
> Cc: Waiman Long <Waiman.Long@hp.com>
> Cc: Jason Low <jason.low2@hp.com>
> Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
> ---
> kernel/futex.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/kernel/futex.c b/kernel/futex.c
> index e6ffe73..0768c68 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -844,9 +844,6 @@ static void wake_futex(struct futex_q *q)
> {
> struct task_struct *p = q->task;
>
> - if (WARN(q->pi_state || q->rt_waiter, "refusing to wake PI futex\n"))
> - return;
> -

This was added deliberately after adding said checks to the callers...
admittedly after a very long debug session I didn't ever want to repeat.
Sometimes warnings are added to make sure we caught everything and later
removed.... sometimes they are added to make sure nothing new ever
breaks this again. Since the failure scenario is non-obvious, unless
this is causing some significant performance issues for you, I'd prefer
this stays.

See commit aa10990e028cac3d5e255711fb9fb47e00700e35 for details.

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