Re: Linux 3.19-rc3

Peter Hurley
2015-01-06T17:58:41Z

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