Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1530460

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

From Dmitry Vyukov <dvyukov@google.com>
Newsgroups linux.kernel
Subject Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE()
Date 2016-11-25 18:30 +0100
Message-ID <sHstY-3tw-27@gated-at.bofh.it> (permalink)
References (6 earlier) <sHo70-CC-41@gated-at.bofh.it> <sHq8N-1SA-9@gated-at.bofh.it> <sHqBQ-2hw-29@gated-at.bofh.it> <sHroe-2Rp-5@gated-at.bofh.it> <sHroe-2Rp-7@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw

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.

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

What about restoring READ/WRITE_ONCE as atomic, and introducing
separate primitives for _non_ atomic loads/stores?
It seems to me that there is just a dozen of cases that don't need
atomicity and where performance is any important (though, some of
these should probably try to write to shared memory less frequently
and save hundreds of cycles, rather than try to save few cycles on
local instructions).

I've compiled kernel with restored size checks in
READ/WRITE/ACCESS_ONCE and the following places seem to expect that
access is actually atomic (while it is not).
But if we don't guarantee that word-sized READ/WRITE_ONCE are atomic,
then I am sure we can find a hundred more of broken places.


arch/x86/entry/vdso/vdso32/../vclock_gettime.c:297:18: note: in
expansion of macro ‘ACCESS_ONCE’
  time_t result = ACCESS_ONCE(gtod->wall_time_sec);

kernel/events/ring_buffer.c:160:10: error: call to
‘__compiletime_assert_160’ declared with attribute error: Need native
word sized stores/loads for atomicity.
   tail = READ_ONCE(rb->user_page->data_tail);

kernel/events/core.c:5145:16: error: call to
‘__compiletime_assert_5145’ declared with attribute error: Need native
word sized stores/loads for atomicity.
   aux_offset = ACCESS_ONCE(rb->user_page->aux_offset);
                ^
kernel/events/core.c:5146:14: error: call to
‘__compiletime_assert_5146’ declared with attribute error: Need native
word sized stores/loads for atomicity.
   aux_size = ACCESS_ONCE(rb->user_page->aux_size);

drivers/cpufreq/cpufreq_governor.c:283:8: error: call to
‘__compiletime_assert_283’ declared with attribute error: Need native
word sized stores/loads for atomicity.
  lst = READ_ONCE(policy_dbs->last_sample_time);
        ^
drivers/cpufreq/cpufreq_governor.c:301:7: error: call to
‘__compiletime_assert_301’ declared with attribute error: Need native
word sized stores/loads for atomicity.
   if (unlikely(lst != READ_ONCE(policy_dbs->last_sample_time))) {

net/core/gen_estimator.c:136:3: error: call to
‘__compiletime_assert_136’ declared with attribute error: Need native
word sized stores/loads for atomicity.
   WRITE_ONCE(e->rate_est->bps, (e->avbps + 0xF) >> 5);
   ^
net/core/gen_estimator.c:142:3: error: call to
‘__compiletime_assert_142’ declared with attribute error: Need native
word sized stores/loads for atomicity.
   WRITE_ONCE(e->rate_est->pps, (e->avpps + 0xF) >> 5);

fs/proc_namespace.c:28:10: error: call to ‘__compiletime_assert_28’
declared with attribute error: Need native word sized stores/loads for
atomicity.
  event = ACCESS_ONCE(ns->event);

drivers/md/dm-stats.c:700:32: error: call to
‘__compiletime_assert_700’ declared with attribute error: Need native
word sized stores/loads for atomicity.
   shared->tmp.sectors[READ] += ACCESS_ONCE(p->sectors[READ]);
                                ^
drivers/md/dm-stats.c:701:33: error: call to
‘__compiletime_assert_701’ declared with attribute error: Need native
word sized stores/loads for atomicity.
   shared->tmp.sectors[WRITE] += ACCESS_ONCE(p->sectors[WRITE]);
                                 ^

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-24 11:30 +0100
  [PATCH 2/3] vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-24 11:30 +0100
    Re: [PATCH 2/3] vringh: kill off ACCESS_ONCE() Christian Borntraeger <borntraeger@de.ibm.com> - 2016-11-24 12:20 +0100
    Re: [PATCH 2/3] vringh: kill off ACCESS_ONCE() Cornelia Huck <cornelia.huck@de.ibm.com> - 2016-11-24 12:40 +0100
    Re: [PATCH 2/3] vringh: kill off ACCESS_ONCE() Jason Wang <jasowang@redhat.com> - 2016-11-25 03:50 +0100
  [PATCH 1/3] tools/virtio: fix READ_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-24 11:30 +0100
    Re: [PATCH 1/3] tools/virtio: fix READ_ONCE() Cornelia Huck <cornelia.huck@de.ibm.com> - 2016-11-24 12:40 +0100
    Re: [PATCH 1/3] tools/virtio: fix READ_ONCE() Jason Wang <jasowang@redhat.com> - 2016-11-25 03:40 +0100
  [PATCH 3/3] tools/virtio: use {READ,WRITE}_ONCE() in uaccess.h Mark Rutland <mark.rutland@arm.com> - 2016-11-24 11:30 +0100
    Re: [PATCH 3/3] tools/virtio: use {READ,WRITE}_ONCE() in uaccess.h Cornelia Huck <cornelia.huck@de.ibm.com> - 2016-11-24 12:40 +0100
    Re: [PATCH 3/3] tools/virtio: use {READ,WRITE}_ONCE() in uaccess.h Jason Wang <jasowang@redhat.com> - 2016-11-25 03:50 +0100
  Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() "Michael S. Tsirkin" <mst@redhat.com> - 2016-11-24 21:40 +0100
    Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-25 12:30 +0100
      Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Christian Borntraeger <borntraeger@de.ibm.com> - 2016-11-25 12:40 +0100
        Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-25 13:30 +0100
          Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Peter Zijlstra <peterz@infradead.org> - 2016-11-25 13:50 +0100
            Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Peter Zijlstra <peterz@infradead.org> - 2016-11-25 13:50 +0100
              Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Boqun Feng <boqun.feng@gmail.com> - 2016-11-25 16:00 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Dmitry Vyukov <dvyukov@google.com> - 2016-11-25 16:30 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-25 17:20 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Peter Zijlstra <peterz@infradead.org> - 2016-11-25 17:20 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-25 17:40 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Christian Borntraeger <borntraeger@de.ibm.com> - 2016-11-25 18:00 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-25 18:30 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Peter Zijlstra <peterz@infradead.org> - 2016-11-25 18:50 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Christian Borntraeger <borntraeger@de.ibm.com> - 2016-11-25 19:50 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() "Michael S. Tsirkin" <mst@redhat.com> - 2016-11-25 22:10 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Christian Borntraeger <borntraeger@de.ibm.com> - 2016-11-25 22:50 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Dmitry Vyukov <dvyukov@google.com> - 2016-11-25 18:30 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-25 18:50 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-25 19:00 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-25 19:10 +0100
                Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-25 19:50 +0100
            Re: [PATCH 0/3] virtio/vringh: kill off ACCESS_ONCE() Mark Rutland <mark.rutland@arm.com> - 2016-11-25 15:40 +0100

csiph-web