lkml.org 
[lkml]   [2016]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2016-03-02 09:01    [W:0.087 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site