On Wed, Mar 2, 2016 at 2:39 PM, Darren Hart <[email protected]> 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
