Messages in this thread | | | Subject | Re: perf events ring buffer memory barrier on powerpc | From | Victor Kaplansky <> | Date | Wed, 30 Oct 2013 19:14:29 +0200 |
| |
Peter Zijlstra <peterz@infradead.org> 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
| |