On 01/06/2015 12:38 PM, Paul E. McKenney wrote:
> On Tue, Jan 06, 2015 at 07:55:39AM -0500, Peter Hurley wrote:
>> [ +cc Paul McKenney ]
>>
>> On 01/06/2015 07:20 AM, Peter Zijlstra wrote:
>>> On Tue, Jan 06, 2015 at 04:01:21AM -0800, Kent Overstreet wrote:
>>>> On Tue, Jan 06, 2015 at 12:48:42PM +0100, Peter Zijlstra wrote:
>>>>>
>>>>>
>>>>> Looking at that closure stuff, why is there an smp_mb() in
>>>>> closure_wake_up() ? Typically wakeup only needs to imply a wmb.
>>>>>
>>>>> Also note that __closure_wake_up() starts with a fully serializing
>>>>> instruction (xchg) and thereby already implies the full barrier.
>>>>
>>>> Probably no good reason, that code is pretty old :)
>>>>
>>>> If I was to hazard a guess, I had my own lockless linked lists before
>>>> llist.h
>>>> existed and perhaps I did it with atomic_xchg() - which was at least
>>>> documented
>>>> to not imply a barrier. I suppose it should just be dropped.
>>>
>>> We (probably me) should probably audit all the atomic_xchg()
>>> implementations and documentation and fix that. I was very much under
>>> the impression it should imply a full barrier (and it certainly does on
>>> x86), the documentation should state the rule that any atomic_ function
>>> that returns a result is fully serializing, therefore, because
>>> atomic_xchg() has a return value, it should too.
>>
>> memory-barriers.txt and atomic_ops.txt appear to contradict each other here,
>> but I think that's because atomic_ops.txt has drifted toward an
>> arch-implementer's POV:
>>
>> 260:atomic_xchg requires explicit memory barriers around the operation.
>>
>> All the serializing atomic operations have descriptions like this.
>
> I am not seeing the contradiction.
>
> You posted the relevant line from atomic_ops.txt. The relevant passage
> from memory-barriers.txt is as follows:
>
> Any atomic operation that modifies some state in memory and
> returns information about the state (old or new) implies an
> SMP-conditional general memory barrier (smp_mb()) on each side
> of the actual operation (with the exception of explicit lock
> operations, described later). These include:
>
> xchg();
> ...
> atomic_xchg(); atomic_long_xchg();
>
> So it appears to me that both documents require full barriers before and
> after any atomic exchange operation in SMP builds. Therefore, any
> SMP-capable architecture that omits these barriers is buggy.
Sure, I understand that, but I think the atomic_ops.txt is easy to
misinterpret.
> So, what am I missing here?
Well, it's a matter of the intended audience. There is a significant
difference between:
static inline int atomic_xchg(atomic_t *v, int new)
{
/* this arch doesn't have serializing xchg() */
smp_mb();
/* arch xchg */
smp_mb();
}
and
smp_mb();
atomic_xchg(&v, 1);
smp_mb();
but both have "explicit memory barriers around the operation."
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
