Re: perf events ring buffer memory barrier on powerpc

From: Victor Kaplansky
Date: Wed Oct 30 2013 - 13:16:08 EST


Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote on 10/30/2013 05:39:31 PM:

> Although I suppose speculative reads are allowed -- they don't have the
> destructive behaviour speculative writes have -- and thus we could in
> fact get reorder issues.

I agree.

>
> But since it is still a dependent load in that we do that @tail vs @head
> comparison before doing other loads, wouldn't a read_barrier_depends()
> be sufficient? Or do we still need a complete rmb?

We need a complete rmb() here IMO. I think there is a fundamental
difference
between load and stores in this aspect. Load are allowed to be hoisted by
compiler or executed speculatively by HW. To prevent load "*(ubuf->data +
tail)"
to be hoisted beyond "ubuf->head" load you would need something like this:

void
ubuf_read(void)
{
u64 head, tail;

tail = ubuf->tail;
head = ACCESS_ONCE(ubuf->head);

/*
* Ensure we read the buffer boundaries before the actual buffer
* data...
*/

while (tail != head) {
smp_read_barrier_depends(); /* for Alpha */
obj = *(ubuf->data + head - 128);
/* process obj */
tail += obj->size;
tail %= ubuf->size;
}

/*
* Ensure all data reads are complete before we issue the
* ubuf->tail update; once that update hits, kbuf_write() can
* observe and overwrite data.
*/
smp_mb(); /* D, matches with A */

ubuf->tail = tail;
}

(note that "head" is part of address calculation of obj load now).

But, even in this demo example some "smp_read_barrier_depends()" before
"obj = *(ubuf->data + head - 100);" is required for architectures
like Alpha. Though, on more sane architectures "smp_read_barrier_depends()"
will be translated to nothing.


Regards,
-- Victor

--
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/