Re: Linux 3.19-rc3

[Paul E. McKenney]

On Tue, Jan 06, 2015 at 12:58:36PM -0500, Peter Hurley wrote:
> 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."

The atomic_ops.txt file is pretty explicit about its intended audience
right at the beginning of the document:

	This document is intended to serve as a guide to Linux port
	maintainers on how to implement atomic counter, bitops, and spinlock
	interfaces properly.

It is intended for people implementing the atomic operations more than
for people making use of them.

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/