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

[Ingo Molnar]

* Darren Hart <dvhart@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > >>>>
> > >>>> That's essential, if you want to hack on futexes.
> > >>>>
> > >>>> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > >>>> Cc: Darren Hart <dvhart@xxxxxxxxxxxxxxx>
> > >>>> Acked-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > >>>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > >>>> Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > >>>> Cc: Mike Galbraith <efault@xxxxxx>
> > >>>> Cc: Jeff Mahoney <jeffm@xxxxxxxx>
> > >>>> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> > >>>> Cc: Scott Norton <scott.norton@xxxxxx>
> > >>>> Cc: Tom Vaden <tom.vaden@xxxxxx>
> > >>>> Cc: Aswin Chandramouleeswaran <aswin@xxxxxx>
> > >>>> Cc: Waiman Long <Waiman.Long@xxxxxx>
> > >>>> Cc: Jason Low <jason.low2@xxxxxx>
> > >>>> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > >>>> Signed-off-by: Davidlohr Bueso <davidlohr@xxxxxx>
> > >>>> ---
> > >>>>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/