* Thomas Gleixner <t...@linutronix.de> wrote: > On Thu, 4 Feb 2016, Davidlohr Bueso wrote: > > > On Thu, 04 Feb 2016, Thomas Gleixner wrote: > > > > > On Wed, 3 Feb 2016, Davidlohr Bueso wrote: > > > > + * We are not calling into get_futex_key_refs() in > > > > file-backed > > > > + * cases, therefore a successful atomic_inc return > > > > below will > > > > + * guarantee that get_futex_key() will continue to > > > > imply MB > > > > (B). > > > > > > Can you please make that "MB (B)" part a bit more outstanding. I really > > > had > > > to > > > search for it. > > > > Hmm as you know this is mostly explained at the begining of the file, and we > > sprinkle MB (B) around the code based on that description. So I'm a bit > > confused > > as to why you don't like like that comment. > > The other "MB (B)" places are more outstanding. It did not spring in my eye > immideately. So it's a pure cosmetic issue.
So I too didn't understand that sentence at first, because the capitalization really throws off quick parsing of that comment, as 'MB' ususally denotes megabytes. So please change it to "mb(); (A)" or so - and I think all of these comments should be changed to use a standard API name for the barrier they imply, as the head of futex.c does: * waiters++; (a) * mb(); (A) <-- paired with -. * | * lock(hash_bucket(futex)); | * | * uval = *futex; | * | *futex = newval; * | sys_futex(WAKE, futex); * | futex_wake(futex); * | * `-------> mb(); (B) Btw., pedantic: shouldn't that be smp_mb()? Futexes don't operate on IO spaces, so on UP they only need compiler barriers. Thanks, Ingo