Messages in this thread | | | Date | Tue, 06 Jan 2015 12:58:36 -0500 | From | Peter Hurley <> | Subject | Re: Linux 3.19-rc3 |
| |
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
| |