Re: [PATCH] perf/ring_buffer: ensure atomicity and order of updates

[Mark Rutland]

On Thu, May 10, 2018 at 02:06:32PM +0100, Mark Rutland wrote:
> -	smp_wmb(); /* B, matches C */
> -	rb->user_page->data_head = head;
> +	smp_store_release(&rb->user_page->data_head, head); /* B, matches C */

> -	rb->user_page->aux_head = rb->aux_head;
> +	smp_store_release(&rb->user_page->aux_head, rb->aux_head);

> -	rb->user_page->aux_head = rb->aux_head;
> +	smp_store_release(&rb->user_page->aux_head, rb->aux_head);


The kbuild test robot has helpfully discovered another latent bug here.

We assume we can make single-copy-atomic accesses to
{aux,data}_{head,tail}, but this isn't necessarily true on 32-bit
architectures, and smp_store_release() rightly complains at build time.

READ_ONCE() and WRITE_ONCE() "helpfully" make a silent fallback to a
memcpy in this case, so we're broken today, regardless of this change.

I suspect that in practice we get single-copy-atomicity for the 32-bit
halves, and sessions likely produce less than 4GiB of ringbuffer data,
so failures would be rare.

I'm not sure how to fix the ABI here. The same issue applies on the
userspace side, so whatever we do we need to fix both sides.

Thanks,
Mark.