search for: 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 instrumented separately). While it's possible to script the bulk o...

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 instrumented separately). While it's possible to script the bulk o...

On 15/02/15 17:30, Raghavendra K T wrote: > --- a/arch/x86/xen/spinlock.c > +++ b/arch/x86/xen/spinlock.c > @@ -41,7 +41,7 @@ static u8 zero_stats; > static inline void check_zero(void) > { > u8 ret; > - u8 old = ACCESS_ONCE(zero_stats); > + u8 old = READ_ONCE(zero_stats); > if (unlikely(old)) { > ret = cmpxchg(&zero_stats, old, 0); > /* This ensures only one fellow resets the stat */ > @@ -112,6 +112,7 @@ __visible void xen_lock_spinning(struct arch_spinlock *lock, __ticket_t want) > st...

On 15/02/15 17:30, Raghavendra K T wrote: > --- a/arch/x86/xen/spinlock.c > +++ b/arch/x86/xen/spinlock.c > @@ -41,7 +41,7 @@ static u8 zero_stats; > static inline void check_zero(void) > { > u8 ret; > - u8 old = ACCESS_ONCE(zero_stats); > + u8 old = READ_ONCE(zero_stats); > if (unlikely(old)) { > ret = cmpxchg(&zero_stats, old, 0); > /* This ensures only one fellow resets the stat */ > @@ -112,6 +112,7 @@ __visible void xen_lock_spinning(struct arch_spinlock *lock, __ticket_t want) > st...

...s->vs_tpg[target]; if (!tv_tpg) { .... return } tv_cmd = vhost_scsi_allocate_cmd(tv_tpg, &v_req, 1) vs->vs_tpg[target] might change after the NULL check and 2) the above line might access tv_tpg from vs->vs_tpg[target]. To prevent 2), use ACCESS_ONCE. Thanks mst for catching this up! Signed-off-by: Asias He <asias at redhat.com> --- drivers/vhost/tcm_vhost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c index 0524267..32d95e3 100644 --- a/drivers/vhost/tcm_vhost...

...s->vs_tpg[target]; if (!tv_tpg) { .... return } tv_cmd = vhost_scsi_allocate_cmd(tv_tpg, &v_req, 1) vs->vs_tpg[target] might change after the NULL check and 2) the above line might access tv_tpg from vs->vs_tpg[target]. To prevent 2), use ACCESS_ONCE. Thanks mst for catching this up! Signed-off-by: Asias He <asias at redhat.com> --- drivers/vhost/tcm_vhost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c index 0524267..32d95e3 100644 --- a/drivers/vhost/tcm_vhost...

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 is certainly the assumption made by many/most users of the *_ONCE() accessors.

...'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...

...'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...

On 11/24/2016 11:25 AM, Mark Rutland wrote: > Despite living under drivers/ vringh.c is also used as part of the userspace > virtio tools. Before we can kill off the ACCESS_ONCE()definition in the tools, > we must convert vringh.c to use {READ,WRITE}_ONCE(). > > This patch does so, along with the required include of <linux/compiler.h> for > the relevant definitions. The userspace tools provide their own definitions in > their own <linux/compiler.h&...

On 2016?11?24? 18:25, Mark Rutland wrote: > Despite living under drivers/ vringh.c is also used as part of the userspace > virtio tools. Before we can kill off the ACCESS_ONCE()definition in the tools, > we must convert vringh.c to use {READ,WRITE}_ONCE(). > > This patch does so, along with the required include of <linux/compiler.h> for > the relevant definitions. The userspace tools provide their own definitions in > their own <linux/compiler.h&g...

Despite living under drivers/ vringh.c is also used as part of the userspace virtio tools. Before we can kill off the ACCESS_ONCE()definition in the tools, we must convert vringh.c to use {READ,WRITE}_ONCE(). This patch does so, along with the required include of <linux/compiler.h> for the relevant definitions. The userspace tools provide their own definitions in their own <linux/compiler.h>. Signed-off-by: Mark...

On Thu, Nov 24, 2016 at 10:36:58PM +0200, Michael S. Tsirkin wrote: > On Thu, Nov 24, 2016 at 10:25:11AM +0000, 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 > > like KTSAN to work correctly (as otherwise reads and writes cannot be > > instrumented separately). >...

On Thu, Nov 24, 2016 at 10:36:58PM +0200, Michael S. Tsirkin wrote: > On Thu, Nov 24, 2016 at 10:25:11AM +0000, 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 > > like KTSAN to work correctly (as otherwise reads and writes cannot be > > instrumented separately). >...

...cketlocks_enabled)) + return TICKET_BATCH_MASK; + else + return TICKET_BATCH_MASK_NATIVE; +} + +static void __ticket_lock_batch_spin(arch_spinlock_t *lock, __ticket_t ticket) +{ + if (static_key_false(&paravirt_ticketlocks_enabled)) { + register struct __raw_tickets inc, new; + + inc.head = ACCESS_ONCE(lock->tickets.head); + barrier(); + for (;;) { + if (!(inc.head & TICKET_LOCK_LOCK_INC)) { + new.head = inc.head | TICKET_LOCK_LOCK_INC; + if (cmpxchg(&lock->tickets.head, inc.head, + new.head) == inc.head) + break; + } + cpu_relax(); + inc.head = ACCESS_ONCE...

...cketlocks_enabled)) + return TICKET_BATCH_MASK; + else + return TICKET_BATCH_MASK_NATIVE; +} + +static void __ticket_lock_batch_spin(arch_spinlock_t *lock, __ticket_t ticket) +{ + if (static_key_false(&paravirt_ticketlocks_enabled)) { + register struct __raw_tickets inc, new; + + inc.head = ACCESS_ONCE(lock->tickets.head); + barrier(); + for (;;) { + if (!(inc.head & TICKET_LOCK_LOCK_INC)) { + new.head = inc.head | TICKET_LOCK_LOCK_INC; + if (cmpxchg(&lock->tickets.head, inc.head, + new.head) == inc.head) + break; + } + cpu_relax(); + inc.head = ACCESS_ONCE...

On Thu, Nov 24, 2016 at 10:25:11AM +0000, 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 > like KTSAN to work correctly (as otherwise reads and writes cannot be > instrumented separately). > > While it's...

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 +0000, 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 >>> like KTSAN to work correctly (as otherwise reads and writes cannot be >>> instrumented separa...

...always the right thing to do. However, the use in __br_mdb_del() is legitimate: They are assigning a pointer to an element from an RCU-protected list, and all elements of this list are already visible to caller. This commit therefore silences these false positives by laundering the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh Triplett. Reported-by: kbuild test robot <fengguang.wu at intel.com> Signed-off-by: Paul E. McKenney <paulmck at linux.vnet.ibm.com> Cc: Stephen Hemminger <stephen at networkplumber.org> Cc: "David S. Miller" <davem at davemlof...

...hings up, just confuses everyone involved. >> >> Let's keep our messy history :) > > By all means! > > You'll first need to cherry-pick these commits: > > 927609d622a3 kernel: tighten rules for ACCESS ONCE > c5b19946eb76 kernel: Fix sparse warning for ACCESS_ONCE > dd36929720f4 kernel: make READ_ONCE() valid on const arguments If you go before 3.19, you will also need 230fa253df63 kernel: Provide READ_ONCE and ASSIGN_ONCE 43239cbe79fc kernel: Change ASSIGN_ONCE(val, x) to WRITE_ONCE(x, val) > > That's the minimum set you will need f...