linux-kernel - Re: [PATCH v4 3/4] futex: Document ordering guarantees

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 for Android: free password hash cracker in your pocket

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

Date:	Fri, 20 Dec 2013 09:17:13 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Darren Hart <dvhart@...ux.intel.com>
Cc:	Randy Dunlap <rdunlap@...radead.org>,
	Davidlohr Bueso <davidlohr@...com>,
	linux-kernel@...r.kernel.org, mingo@...hat.com,
	peterz@...radead.org, tglx@...utronix.de,
	paulmck@...ux.vnet.ibm.com, efault@....de, jeffm@...e.com,
	torvalds@...ux-foundation.org, jason.low2@...com,
	Waiman.Long@...com, tom.vaden@...com, scott.norton@...com,
	aswin@...com
Subject: Re: [PATCH v4 3/4] futex: Document ordering guarantees


* Darren Hart <dvhart@...ux.intel.com> wrote:

> On Thu, 2013-12-19 at 15:05 -0800, Randy Dunlap wrote:
> > On 12/19/13 13:07, Darren Hart wrote:
> > > On Thu, 2013-12-19 at 13:00 -0800, Davidlohr Bueso wrote:
> > >> On Thu, 2013-12-19 at 12:51 -0800, Randy Dunlap wrote:
> > >>> On 12/19/13 12:45, Davidlohr Bueso wrote:
> > >>>> From: Thomas Gleixner <tglx@...utronix.de>
> > >>>>
> > >>>> That's essential, if you want to hack on futexes.
> > >>>>
> > >>>> Cc: Ingo Molnar <mingo@...nel.org>
> > >>>> Cc: Darren Hart <dvhart@...ux.intel.com>
> > >>>> Acked-by: Peter Zijlstra <peterz@...radead.org>
> > >>>> Cc: Thomas Gleixner <tglx@...utronix.de>
> > >>>> Cc: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> > >>>> Cc: Mike Galbraith <efault@....de>
> > >>>> Cc: Jeff Mahoney <jeffm@...e.com>
> > >>>> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> > >>>> Cc: Scott Norton <scott.norton@...com>
> > >>>> Cc: Tom Vaden <tom.vaden@...com>
> > >>>> Cc: Aswin Chandramouleeswaran <aswin@...com>
> > >>>> Cc: Waiman Long <Waiman.Long@...com>
> > >>>> Cc: Jason Low <jason.low2@...com>
> > >>>> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> > >>>> Signed-off-by: Davidlohr Bueso <davidlohr@...com>
> > >>>> ---
> > >>>>  kernel/futex.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >>>>  1 file changed, 57 insertions(+)
> > >>>>
> > >>>> diff --git a/kernel/futex.c b/kernel/futex.c
> > >>>> index 577481d..af1fc31 100644
> > >>>> --- a/kernel/futex.c
> > >>>> +++ b/kernel/futex.c
> > >>>> @@ -69,6 +69,63 @@
> > >>>>  
> > >>>>  #include "locking/rtmutex_common.h"
> > >>>>  
> > >>>> +/*
> > >>>> + * Basic futex operation and ordering guarantees:
> > >>>> + *
> > >>>> + * The waiter reads the futex value in user space and calls
> > >>>> + * futex_wait(). It computes the hash bucket and acquires the hash
> > >>>
> > >>> does                It
> > >>> refer to "the waiter" or to futex_wait()?
> > >>> I read it as referring to the waiter, but ISTM that the comments are using It
> > >>> to refer to futex_wait()... ???
> > >>>
> > >>
> > >> Yes, it refers to futex_wait(), which IMO in a futex context is pretty
> > >> clear.
> > > 
> > > 
> > > Agreed.
> > > 
> > > 
> > 
> > I'll just have to get by with disagreeing with both of you then.
> > I say that the pronoun's antecedent is ambiguous (unclear).
> > 
> 
> You are correct from a purely grammatical perspective, no argument 
> there. I just feel that the comment is clear enough from the context 
> - although perhaps people that hack on futexes should be excluded 
> from having an opinion here as we are biased :-)

It doesn't really matter that to _you_ (the futex expert) the sentence 
seems unambiguous, such comments are mostly meant for new people 
reading the code, such as Randy!

So guys please fix the comment instead of arguing about it.

Thanks,

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