Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE()

[Mark Rutland <mark.rutland@arm.com>]

On Fri, Nov 25, 2016 at 06:28:53PM +0100, Dmitry Vyukov wrote:
> On Fri, Nov 25, 2016 at 5:17 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> >> > What are use cases for such primitive that won't be OK with "read once
> >> > _and_ atomically"?
> >>
> >> I have none to hand.
> >
> > Whatever triggers the __builtin_memcpy() paths, and even the size==8
> > paths on 32bit.
> >
> > You could put a WARN in there to easily find them.
> >
> > The advantage of introducing the SINGLE_{LOAD,STORE}() helpers is that
> > they compiletime validate this the size is 'right' and can runtime check
> > alignment constraints.
> >
> > IE, they are strictly stronger than {READ,WRITE}_ONCE().
> 
> Uh, so, READ/WRITE_ONCE are non-atomic now. I missed that.

Yes, but *only* for types larger than word size. That has *always* been
the case.

It's still assumed that *_ONCE are single-copy-atomic for word size (or
smaller). Some architectures may also provide that guarnatee for
accesses larger than word size (e.g. 32-bit ARM w/ LPAE).

... It's just that as things stand we can't put checks in *_ONCE() for
the access size, since they're *also* used for larger accesses that
don't need atomicity.

> If READ/WRITE_ONCE are non-atomic, half of kernel is broken. All these
> loads of flags, ringbuffer positions, pointers, etc are broken.

Most of these will be fine, as above.

> What about restoring READ/WRITE_ONCE as atomic, and introducing
> separate primitives for _non_ atomic loads/stores?

Having a separate *_ONCE_TEARABLE() would certainly limit the number of
things we have to fix up, and would also make it clear that atomicity is
not expected.

... but we might have to go with SINGLE_*() if we can't convince Linus.

Thanks,
Mark.