[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20131101161129.GU16117@laptop.programming.kicks-ass.net>
Date: Fri, 1 Nov 2013 17:11:29 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc: Victor Kaplansky <VICTORK@...ibm.com>,
Oleg Nesterov <oleg@...hat.com>,
Anton Blanchard <anton@...ba.org>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Frederic Weisbecker <fweisbec@...il.com>,
LKML <linux-kernel@...r.kernel.org>,
Linux PPC dev <linuxppc-dev@...abs.org>,
Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>,
Michael Ellerman <michael@...erman.id.au>,
Michael Neuling <mikey@...ling.org>
Subject: Re: perf events ring buffer memory barrier on powerpc
On Wed, Oct 30, 2013 at 11:40:15PM -0700, Paul E. McKenney wrote:
> > void kbuf_write(int sz, void *buf)
> > {
> > u64 tail = ACCESS_ONCE(ubuf->tail); /* last location userspace read */
> > u64 offset = kbuf->head; /* we already know where we last wrote */
> > u64 head = offset + sz;
> >
> > if (!space(tail, offset, head)) {
> > /* discard @buf */
> > return;
> > }
> >
> > /*
> > * Ensure that if we see the userspace tail (ubuf->tail) such
> > * that there is space to write @buf without overwriting data
> > * userspace hasn't seen yet, we won't in fact store data before
> > * that read completes.
> > */
> >
> > smp_mb(); /* A, matches with D */
> >
> > write(kbuf->data + offset, buf, sz);
> > kbuf->head = head % kbuf->size;
> >
> > /*
> > * Ensure that we write all the @buf data before we update the
> > * userspace visible ubuf->head pointer.
> > */
> > smp_wmb(); /* B, matches with C */
> >
> > ubuf->head = kbuf->head;
> > }
> > Now the whole crux of the question is if we need barrier A at all, since
> > the STORES issued by the @buf writes are dependent on the ubuf->tail
> > read.
>
> The dependency you are talking about is via the "if" statement?
> Even C/C++11 is not required to respect control dependencies.
But surely we must be able to make it so; otherwise you'd never be able
to write:
void *ptr = obj1;
void foo(void)
{
/* create obj2, obj3 */
smp_wmb(); /* ensure the objs are complete */
/* expose either obj2 or obj3 */
if (x)
ptr = obj2;
else
ptr = obj3;
/* free the unused one */
if (x)
free(obj3);
else
free(obj2);
}
Earlier you said that 'volatile' or '__atomic' avoids speculative
writes; so would:
volatile void *ptr = obj1;
Make the compiler respect control dependencies again? If so, could we
somehow mark that !space() condition volatile?
Currently the above would be considered a valid pattern. But you're
saying its not because the compiler is free to expose both obj2 and obj3
(for however short a time) and thus the free of the 'unused' object is
incorrect and can cause use-after-free.
In fact; how can we be sure that:
void *ptr = NULL;
void bar(void)
{
void *obj = malloc(...);
/* fill obj */
if (!err)
rcu_assign_pointer(ptr, obj);
else
free(obj);
}
Does not get 'optimized' into:
void bar(void)
{
void *obj = malloc(...);
void *old_ptr = ptr;
/* fill obj */
rcu_assign_pointer(ptr, obj);
if (err) { /* because runtime profile data says this is unlikely */
ptr = old_ptr;
free(obj);
}
}
We _MUST_ be able to rely on control flow, otherwise me might as well
all go back to writing kernels in asm.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists