Messages in this thread | | | From | Jianyu Zhan <> | Date | Wed, 2 Mar 2016 15:12:51 +0800 | Subject | Re: [PATCH] futex: replace bare barrier() with a READ_ONCE() |
| |
On Wed, Mar 2, 2016 at 2:39 PM, Darren Hart <dvhart@infradead.org> wrote: > I believe you are correct with respect to the retry and while condition being an > appropriate place for the application of READ_ONCE. The question is why is this > preferred to the existing barrier()? I suggest: > > While barrier() is a fairly brute force general application of a compiler > barrier, READ_ONCE() is very specific, it targets only operations dealing with > the specified variable. As such, its application both clearly identifies the > volatile variable and frees the compiler to make optimizations a more general > barrier would forbid. > >
Yep, beside the informative point, the more specifics of READ_ONCE over barrier is what I meant "lightweight", I missed emphasizing this point. Thanks for pointing it.
I will respin this patch to reflect this.
>> >> >> > As for #2... >> > >> >> 2. For the second problem I memtioned, yes, it is theoretical, and >> >> it is also due to q->lock_ptr can change between >> >> the test of nullness of q->lock_ptr and the lock on q->lock_ptr. >> >> >> >> the code is >> >> >> >> retry: >> >> lock_ptr = q->lock_ptr; >> >> if (lock_ptr != 0) { >> >> spin_lock(lock_ptr) >> >> if (unlikely(lock_ptr != q->lock_ptr)) { >> >> spin_unlock(lock_ptr); >> >> goto retry; >> >> } >> >> ... >> >> } >> >> >> >> which is effectively the same as : >> >> >> >> while (lock_ptr = q->lock_ptr) { >> >> spin_lock(lock_ptr) >> >> if (unlikely(lock_ptr != q->lock_ptr)) { >> >> spin_unlock(lock_ptr); >> >> goto retry; >> >> } >> >> ... >> >> } >> >> >> >> This might cause the compiler load the q->lock_ptr once and use it >> >> many times, quoted from > > Which is already covered by the barrier() in place today as a more general > compiler barrier. > > Your argument is then simply that READ_ONCE is a more specific solution to the > problem. >
Yep. And after re-thinking, I am now less convinced in this second argument, since it involves a comparison of q->lock_ptr in the loop body, so this may defeat any attempts that compilers try to optimize the load out of the loop, even without a READ_ONCE().
But I will also incorporate this in the second submission for review .
Regards, Jianyu Zhan
| |