linux-kernel - Re: [PATCH -v4 03/10] futex: Cleanup variable names for futex_top

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170222140700.GU6515@twins.programming.kicks-ass.net>
Date:   Wed, 22 Feb 2017 15:07:00 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Darren Hart <dvhart@...radead.org>
Cc:     tglx@...utronix.de, mingo@...nel.org, juri.lelli@....com,
        rostedt@...dmis.org, xlpang@...hat.com, bigeasy@...utronix.de,
        linux-kernel@...r.kernel.org, mathieu.desnoyers@...icios.com,
        jdesfossez@...icios.com, bristot@...hat.com
Subject: Re: [PATCH -v4 03/10] futex: Cleanup variable names for
 futex_top_waiter()

On Fri, Dec 16, 2016 at 04:13:10PM -0800, Darren Hart wrote:
> On Tue, Dec 13, 2016 at 09:36:41AM +0100, Peter Zijlstra wrote:
> > futex_top_waiter() returns the top-waiter on the pi_mutex. Assinging
> > this to a variable 'match' totally obscures the code.
> > 
> 
> Yes please.
> 
> One wording nit...
> 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> > ---
> >  kernel/futex.c |   30 +++++++++++++++---------------
> >  1 file changed, 15 insertions(+), 15 deletions(-)
> > 
> > --- a/kernel/futex.c
> > +++ b/kernel/futex.c
> 
> > @@ -1317,11 +1317,11 @@ static int wake_futex_pi(u32 __user *uad
> >  
> >  	/*
> >  	 * It is possible that the next waiter (the one that brought
> > -	 * this owner to the kernel) timed out and is no longer
> > +	 * top_waiter owner to the kernel) timed out and is no longer
> >  	 * waiting on the lock.
> >  	 */
> 
> This breaks my parser (and did before too).
> 
> Consider:
> 
> /*
>  * It is possible that the next waiter (that caused top_waiter to call
>  * into the kernel) has since timed out and is no longer waiting on the
>  * lock.
>  */
> 
> Is that clearer?

No :-)

Fear not, this wee comment here is what the next many patches are about.
This is the 'little' hole that opens up sooo much pain. In any case,
fear not, the comment will die later on.