Messages in this thread | | | Date | Tue, 28 Apr 2015 13:33:23 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH 2/2] [PATCH] sched: Add smp_rmb() in task rq locking cycles |
| |
On Tue, Apr 28, 2015 at 04:32:35PM +0200, Peter Zijlstra wrote: > On Sat, Apr 25, 2015 at 12:56:03PM -0700, Paul E. McKenney wrote: > > smp: Make control dependencies work on Alpha, improve documentation > > > > The current formulation of control dependencies fails on DEC Alpha, which > > does not respect dependencies of any kind unless an explicit memory is > > + barrier ?
Good catch, fixed!
> > provided. This means that the current fomulation of control dependencies > > fails on Alpha. This commit therefore creates a READ_ONCE_CTRL() that > > has the same overhead on non-Alpha systems, but causes Alpha to produce > > the needed ordering. This commit also applies READ_ONCE_CTRL() to the > > one known use of control dependencies. > > > > Use of READ_ONCE_CTRL() also has the beneficial effect of adding a bit > > of self-documentation to control dependencies. > > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Applied, thank you!
Thanx, Paul
> > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > > index 1b45e4a0519b..a57eacde2b84 100644 > > --- a/include/linux/compiler.h > > +++ b/include/linux/compiler.h > > @@ -264,6 +264,22 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s > > #define WRITE_ONCE(x, val) \ > > ({ typeof(x) __val = (val); __write_once_size(&(x), &__val, sizeof(__val)); __val; }) > > > > +/** > > + * READ_ONCE_CTRL - Read a value heading a control dependency > > + * @x: The value to be read, heading the control dependency > > + * > > + * Control dependencies are tricky. See Documentation/memory-barriers.txt > > + * for important information on how to use them. Note that in many cases, > > + * use of smp_load_acquire() will be much simpler. Control dependencies > > + * should be avoided except on the hottest of hotpaths. > > + */ > > +#define READ_ONCE_CTRL(x) \ > > +({ \ > > + typeof(x) __val = READ_ONCE(x); \ > > + smp_read_barrier_depends(); /* Enforce control dependency. */ \ > > + __val; \ > > +}) > > + > > #endif /* __KERNEL__ */ > > > > #endif /* __ASSEMBLY__ */ > > We mostly try and align the \ for multi-line macros. > > > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c > > index eadb95ce7aac..67548d5de4cb 100644 > > --- a/kernel/events/ring_buffer.c > > +++ b/kernel/events/ring_buffer.c > > @@ -141,7 +141,7 @@ int perf_output_begin(struct perf_output_handle *handle, > > perf_output_get_handle(handle); > > > > do { > > - tail = ACCESS_ONCE(rb->user_page->data_tail); > > + tail = READ_ONCE_CTRL(rb->user_page->data_tail); > > offset = head = local_read(&rb->head); > > if (!rb->overwrite && > > unlikely(CIRC_SPACE(head, tail, perf_data_size(rb)) < size)) > > Right. I could not remember any other current usage. >
| |