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

On 11/25/2016 10:08 PM, Michael S. Tsirkin wrote: > On Fri, Nov 25, 2016 at 05:49:45PM +0100, Christian Borntraeger wrote: >> On 11/25/2016 05:17 PM, Peter Zijlstra wrote: >>> On Fri, Nov 25, 2016 at 04:10:04PM +, Mark Rutland wrote: On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov

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

On Fri, Nov 25, 2016 at 05:49:45PM +0100, Christian Borntraeger wrote: > On 11/25/2016 05:17 PM, Peter Zijlstra wrote: > > On Fri, Nov 25, 2016 at 04:10:04PM +, Mark Rutland wrote: > >> On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov wrote: > > > >>> What are use cases for such

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

On Fri, Nov 25, 2016 at 10:07 AM, Mark Rutland wrote: > On Fri, Nov 25, 2016 at 09:52:50AM -0800, Linus Torvalds wrote: >> READ/WRITE_ONCE() are atomic *WHEN*THAT*IS*POSSIBLE*. > >> But sometimes it's not going to be atomic. > > That's the problem. It has never really been

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

On 11/25/2016 06:28 PM, Mark Rutland wrote: > On Fri, Nov 25, 2016 at 05:49:45PM +0100, Christian Borntraeger wrote: >> On 11/25/2016 05:17 PM, Peter Zijlstra wrote: >>> On Fri, Nov 25, 2016 at 04:10:04PM +, Mark Rutland wrote: On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov

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

On Fri, Nov 25, 2016 at 09:52:50AM -0800, Linus Torvalds wrote: > READ/WRITE_ONCE() are atomic *WHEN*THAT*IS*POSSIBLE*. > But sometimes it's not going to be atomic. That's the problem. Common code may rely on something being atomic when that's only true on a subset of platforms. On others, it's

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

On Fri, Nov 25, 2016 at 9:28 AM, Dmitry Vyukov wrote: On Fri, Nov 25, 2016 at 5:17 PM, Peter Zijlstra wrote: >> >> IE, they are strictly stronger than {READ,WRITE}_ONCE(). No, they are strictly bullshit. Stop this idiocy. We went through this once

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

On Fri, Nov 25, 2016 at 06:28:53PM +0100, Dmitry Vyukov wrote: > On Fri, Nov 25, 2016 at 5:17 PM, Peter Zijlstra wrote: > >> > What are use cases for such primitive that won't be OK with "read once > >> > _and_ atomically"? > >> > >> I have none to hand. > > > > Whatever

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

On Fri, Nov 25, 2016 at 05:28:01PM +, Mark Rutland wrote: > On Fri, Nov 25, 2016 at 05:49:45PM +0100, Christian Borntraeger wrote: > > On 11/25/2016 05:17 PM, Peter Zijlstra wrote: > > There were several cases that I found during writing the *ONCE stuff. > > For example there are some 32bit

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

On Fri, Nov 25, 2016 at 5:17 PM, Peter Zijlstra 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. >

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

On Fri, Nov 25, 2016 at 05:49:45PM +0100, Christian Borntraeger wrote: > On 11/25/2016 05:17 PM, Peter Zijlstra wrote: > > On Fri, Nov 25, 2016 at 04:10:04PM +, Mark Rutland wrote: > >> On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov wrote: > > > >>> What are use cases for such

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

On 11/25/2016 05:17 PM, Peter Zijlstra wrote: > On Fri, Nov 25, 2016 at 04:10:04PM +, Mark Rutland wrote: >> On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov wrote: > >>> What are use cases for such primitive that won't be OK with "read once >>> _and_ atomically"? >> >> I have none to

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

On Fri, Nov 25, 2016 at 05:17:09PM +0100, Peter Zijlstra wrote: > On Fri, Nov 25, 2016 at 04:10:04PM +, Mark Rutland wrote: > > On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov wrote: > > > > What are use cases for such primitive that won't be OK with "read once > > > _and_

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

On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov wrote: > > READ/WRITE_ONCE imply atomicity. Even if their names don't spell it (a > function name doesn't have to spell all of its guarantees). Most of > the uses of READ/WRITE_ONCE will be broken if they are not atomic. In practice, this

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

On Fri, Nov 25, 2016 at 04:10:04PM +, Mark Rutland wrote: > On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov 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()

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

On Fri, Nov 25, 2016 at 3:56 PM, Boqun Feng wrote: > On Fri, Nov 25, 2016 at 01:44:04PM +0100, Peter Zijlstra wrote: >> On Fri, Nov 25, 2016 at 01:40:44PM +0100, Peter Zijlstra wrote: >> > #define SINGLE_LOAD(x) \ >> > {(

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

On Fri, Nov 25, 2016 at 01:44:04PM +0100, Peter Zijlstra wrote: > On Fri, Nov 25, 2016 at 01:40:44PM +0100, Peter Zijlstra wrote: > > #define SINGLE_LOAD(x) \ > > {( \ > >

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

On Fri, Nov 25, 2016 at 01:40:44PM +0100, Peter Zijlstra wrote: > On Fri, Nov 25, 2016 at 12:23:56PM +, Mark Rutland wrote: > > Naming will be problematic; calling them ATOMIC_* makes tham sound like > > they work on atomic_t. That and I have no idea how to ensure correct > > usage tree-wide;

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

On Fri, Nov 25, 2016 at 01:40:44PM +0100, Peter Zijlstra wrote: > #define SINGLE_LOAD(x)\ > {(\ > compiletime_assert_atomic_type(typeof(x)); \ Should be:

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

On Fri, Nov 25, 2016 at 12:23:56PM +, Mark Rutland wrote: > Naming will be problematic; calling them ATOMIC_* makes tham sound like > they work on atomic_t. That and I have no idea how to ensure correct > usage tree-wide; I'm not sure if/how Coccinelle can help. > > Peter, thoughts?

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

On Fri, Nov 25, 2016 at 12:33:48PM +0100, Christian Borntraeger wrote: > On 11/25/2016 12:22 PM, Mark Rutland wrote: > > On Thu, Nov 24, 2016 at 10:36:58PM +0200, Michael S. Tsirkin wrote: > >> Though I really question the whole _ONCE APIs esp with > >> aggregate types - these seem to generate a

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

On 11/25/2016 12:22 PM, Mark Rutland wrote: > On Thu, Nov 24, 2016 at 10:36:58PM +0200, Michael S. Tsirkin wrote: >> On Thu, Nov 24, 2016 at 10:25:11AM +, Mark Rutland wrote: >>> For several reasons, it would be beneficial to kill off ACCESS_ONCE() >>> tree-wide, in favour of

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

On Thu, Nov 24, 2016 at 10:36:58PM +0200, Michael S. Tsirkin wrote: > On Thu, Nov 24, 2016 at 10:25:11AM +, Mark Rutland wrote: > > For several reasons, it would be beneficial to kill off ACCESS_ONCE() > > tree-wide, in favour of {READ,WRITE}_ONCE(). These work with aggregate > > types, > >

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

On Thu, Nov 24, 2016 at 10:25:11AM +, Mark Rutland wrote: > For several reasons, it would be beneficial to kill off ACCESS_ONCE() > tree-wide, in favour of {READ,WRITE}_ONCE(). These work with aggregate types, > more obviously document their intended behaviour, and are necessary for tools >

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

For several reasons, it would be beneficial to kill off ACCESS_ONCE() tree-wide, in favour of {READ,WRITE}_ONCE(). These work with aggregate types, more obviously document their intended behaviour, and are necessary for tools like KTSAN to work correctly (as otherwise reads and writes cannot be