* [PATCH 0/8] current ACCESS_ONCE patch queue
@ 2015-01-15 8:58 Christian Borntraeger
2015-01-15 8:58 ` [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE Christian Borntraeger
` (8 more replies)
0 siblings, 9 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm,
Christian Borntraeger
Folks,
fyi, this is my current patch queue for the next merge window. It
does contain a patch that will disallow ACCESS_ONCE on non-scalar
types.
The tree is part of linux-next and can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/borntraeger/linux.git linux-next
Christian Borntraeger (7):
ppc/kvm: Replace ACCESS_ONCE with READ_ONCE
ppc/hugetlbfs: Replace ACCESS_ONCE with READ_ONCE
x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE
mm/gup: Replace ACCESS_ONCE with READ_ONCE
kernel: tighten rules for ACCESS ONCE
kernel: Fix sparse warning for ACCESS_ONCE
Guenter Roeck (1):
next: sh: Fix compile error
arch/powerpc/kvm/book3s_hv_rm_xics.c | 8 ++++----
arch/powerpc/kvm/book3s_xics.c | 16 ++++++++--------
arch/powerpc/mm/hugetlbpage.c | 4 ++--
arch/sh/mm/gup.c | 2 +-
arch/x86/include/asm/spinlock.h | 2 +-
arch/x86/xen/p2m.c | 2 +-
include/linux/compiler.h | 21 ++++++++++++++++-----
mm/gup.c | 2 +-
8 files changed, 34 insertions(+), 23 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
@ 2015-01-15 8:58 ` Christian Borntraeger
2015-01-15 23:09 ` Michael Ellerman
2015-01-15 8:58 ` [PATCH 2/8] ppc/hugetlbfs: " Christian Borntraeger
` (7 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm,
Christian Borntraeger
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the ppc/kvm code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/powerpc/kvm/book3s_hv_rm_xics.c | 8 ++++----
arch/powerpc/kvm/book3s_xics.c | 16 ++++++++--------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c b/arch/powerpc/kvm/book3s_hv_rm_xics.c
index 7b066f6..7c22997 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_xics.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c
@@ -152,7 +152,7 @@ static void icp_rm_down_cppr(struct kvmppc_xics *xics, struct kvmppc_icp *icp,
* in virtual mode.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
/* Down_CPPR */
new_state.cppr = new_cppr;
@@ -211,7 +211,7 @@ unsigned long kvmppc_rm_h_xirr(struct kvm_vcpu *vcpu)
* pending priority
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
xirr = old_state.xisr | (((u32)old_state.cppr) << 24);
if (!old_state.xisr)
@@ -277,7 +277,7 @@ int kvmppc_rm_h_ipi(struct kvm_vcpu *vcpu, unsigned long server,
* whenever the MFRR is made less favored.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
/* Set_MFRR */
new_state.mfrr = mfrr;
@@ -352,7 +352,7 @@ int kvmppc_rm_h_cppr(struct kvm_vcpu *vcpu, unsigned long cppr)
icp_rm_clr_vcpu_irq(icp->vcpu);
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
reject = 0;
new_state.cppr = cppr;
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index 807351f..a4a8d9f 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -327,7 +327,7 @@ static bool icp_try_to_deliver(struct kvmppc_icp *icp, u32 irq, u8 priority,
icp->server_num);
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
*reject = 0;
@@ -512,7 +512,7 @@ static void icp_down_cppr(struct kvmppc_xics *xics, struct kvmppc_icp *icp,
* in virtual mode.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
/* Down_CPPR */
new_state.cppr = new_cppr;
@@ -567,7 +567,7 @@ static noinline unsigned long kvmppc_h_xirr(struct kvm_vcpu *vcpu)
* pending priority
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
xirr = old_state.xisr | (((u32)old_state.cppr) << 24);
if (!old_state.xisr)
@@ -634,7 +634,7 @@ static noinline int kvmppc_h_ipi(struct kvm_vcpu *vcpu, unsigned long server,
* whenever the MFRR is made less favored.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
/* Set_MFRR */
new_state.mfrr = mfrr;
@@ -679,7 +679,7 @@ static int kvmppc_h_ipoll(struct kvm_vcpu *vcpu, unsigned long server)
if (!icp)
return H_PARAMETER;
}
- state = ACCESS_ONCE(icp->state);
+ state = READ_ONCE(icp->state);
kvmppc_set_gpr(vcpu, 4, ((u32)state.cppr << 24) | state.xisr);
kvmppc_set_gpr(vcpu, 5, state.mfrr);
return H_SUCCESS;
@@ -721,7 +721,7 @@ static noinline void kvmppc_h_cppr(struct kvm_vcpu *vcpu, unsigned long cppr)
BOOK3S_INTERRUPT_EXTERNAL_LEVEL);
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
reject = 0;
new_state.cppr = cppr;
@@ -885,7 +885,7 @@ static int xics_debug_show(struct seq_file *m, void *private)
if (!icp)
continue;
- state.raw = ACCESS_ONCE(icp->state.raw);
+ state.raw = READ_ONCE(icp->state.raw);
seq_printf(m, "cpu server %#lx XIRR:%#x PPRI:%#x CPPR:%#x MFRR:%#x OUT:%d NR:%d\n",
icp->server_num, state.xisr,
state.pending_pri, state.cppr, state.mfrr,
@@ -1082,7 +1082,7 @@ int kvmppc_xics_set_icp(struct kvm_vcpu *vcpu, u64 icpval)
* the ICS states before the ICP states.
*/
do {
- old_state = ACCESS_ONCE(icp->state);
+ old_state = READ_ONCE(icp->state);
if (new_state.mfrr <= old_state.mfrr) {
resend = false;
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/8] ppc/hugetlbfs: Replace ACCESS_ONCE with READ_ONCE
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
2015-01-15 8:58 ` [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE Christian Borntraeger
@ 2015-01-15 8:58 ` Christian Borntraeger
2015-01-15 8:58 ` [PATCH 3/8] x86/xen/p2m: " Christian Borntraeger
` (6 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm,
Christian Borntraeger
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the ppc/hugetlbfs code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/powerpc/mm/hugetlbpage.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 5ff4e07..620d0ec 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -978,7 +978,7 @@ pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea, unsigned *shift
*/
pdshift = PUD_SHIFT;
pudp = pud_offset(&pgd, ea);
- pud = ACCESS_ONCE(*pudp);
+ pud = READ_ONCE(*pudp);
if (pud_none(pud))
return NULL;
@@ -990,7 +990,7 @@ pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea, unsigned *shift
else {
pdshift = PMD_SHIFT;
pmdp = pmd_offset(&pud, ea);
- pmd = ACCESS_ONCE(*pmdp);
+ pmd = READ_ONCE(*pmdp);
/*
* A hugepage collapse is captured by pmd_none, because
* it mark the pmd none and do a hpte invalidate.
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
2015-01-15 8:58 ` [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE Christian Borntraeger
2015-01-15 8:58 ` [PATCH 2/8] ppc/hugetlbfs: " Christian Borntraeger
@ 2015-01-15 8:58 ` Christian Borntraeger
2015-01-15 9:26 ` Jürgen Groß
2015-01-15 10:43 ` [Xen-devel] " David Vrabel
2015-01-15 8:58 ` [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE Christian Borntraeger
` (5 subsequent siblings)
8 siblings, 2 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm,
Christian Borntraeger
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the p2m code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/x86/xen/p2m.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index edbc7a6..cb71016 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -554,7 +554,7 @@ static bool alloc_p2m(unsigned long pfn)
mid_mfn = NULL;
}
- p2m_pfn = pte_pfn(ACCESS_ONCE(*ptep));
+ p2m_pfn = pte_pfn(READ_ONCE(*ptep));
if (p2m_pfn == PFN_DOWN(__pa(p2m_identity)) ||
p2m_pfn == PFN_DOWN(__pa(p2m_missing))) {
/* p2m leaf page is missing */
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
` (2 preceding siblings ...)
2015-01-15 8:58 ` [PATCH 3/8] x86/xen/p2m: " Christian Borntraeger
@ 2015-01-15 8:58 ` Christian Borntraeger
2015-01-15 19:38 ` Oleg Nesterov
2015-01-15 8:58 ` [PATCH 5/8] mm/gup: Replace ACCESS_ONCE with READ_ONCE Christian Borntraeger
` (4 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm,
Christian Borntraeger, Oleg Nesterov
commit 78bff1c8684f ("x86/ticketlock: Fix spin_unlock_wait() livelock")
introduced another ACCESS_ONCE case in x86 spinlock.h.
Change that as well.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Oleg Nesterov <oleg@redhat.com>
---
arch/x86/include/asm/spinlock.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index 625660f..9264f0f 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
__ticket_t head = ACCESS_ONCE(lock->tickets.head);
for (;;) {
- struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
+ struct __raw_tickets tmp = READ_ONCE(lock->tickets);
/*
* We need to check "unlocked" in a loop, tmp.head == head
* can be false positive because of overflow.
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/8] mm/gup: Replace ACCESS_ONCE with READ_ONCE
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
` (3 preceding siblings ...)
2015-01-15 8:58 ` [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE Christian Borntraeger
@ 2015-01-15 8:58 ` Christian Borntraeger
2015-01-15 8:58 ` [PATCH 6/8] kernel: tighten rules for ACCESS ONCE Christian Borntraeger
` (3 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm,
Christian Borntraeger
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Fixup gup_pmd_range.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
mm/gup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/gup.c b/mm/gup.c
index a900759..bed30efa 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -926,7 +926,7 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
pmdp = pmd_offset(&pud, addr);
do {
- pmd_t pmd = ACCESS_ONCE(*pmdp);
+ pmd_t pmd = READ_ONCE(*pmdp);
next = pmd_addr_end(addr, end);
if (pmd_none(pmd) || pmd_trans_splitting(pmd))
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 6/8] kernel: tighten rules for ACCESS ONCE
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
` (4 preceding siblings ...)
2015-01-15 8:58 ` [PATCH 5/8] mm/gup: Replace ACCESS_ONCE with READ_ONCE Christian Borntraeger
@ 2015-01-15 8:58 ` Christian Borntraeger
2015-01-15 8:58 ` [PATCH 7/8] next: sh: Fix compile error Christian Borntraeger
` (2 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm,
Christian Borntraeger
Now that all non-scalar users of ACCESS_ONCE have been converted
to READ_ONCE or ASSIGN once, lets tighten ACCESS_ONCE to only
work on scalar types.
This variant was proposed by Alexei Starovoitov.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
include/linux/compiler.h | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index a1c81f8..5e186bf 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -447,12 +447,23 @@ static __always_inline void __assign_once_size(volatile void *p, void *res, int
* to make the compiler aware of ordering is to put the two invocations of
* ACCESS_ONCE() in different C statements.
*
- * This macro does absolutely -nothing- to prevent the CPU from reordering,
- * merging, or refetching absolutely anything at any time. Its main intended
- * use is to mediate communication between process-level code and irq/NMI
- * handlers, all running on the same CPU.
+ * ACCESS_ONCE will only work on scalar types. For union types, ACCESS_ONCE
+ * on a union member will work as long as the size of the member matches the
+ * size of the union and the size is smaller than word size.
+ *
+ * The major use cases of ACCESS_ONCE used to be (1) Mediating communication
+ * between process-level code and irq/NMI handlers, all running on the same CPU,
+ * and (2) Ensuring that the compiler does not fold, spindle, or otherwise
+ * mutilate accesses that either do not require ordering or that interact
+ * with an explicit memory barrier or atomic instruction that provides the
+ * required ordering.
+ *
+ * If possible use READ_ONCE/ASSIGN_ONCE instead.
*/
-#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
+#define __ACCESS_ONCE(x) ({ \
+ __maybe_unused typeof(x) __var = 0; \
+ (volatile typeof(x) *)&(x); })
+#define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
/* Ignore/forbid kprobes attach on very low level functions marked by this attribute: */
#ifdef CONFIG_KPROBES
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 7/8] next: sh: Fix compile error
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
` (5 preceding siblings ...)
2015-01-15 8:58 ` [PATCH 6/8] kernel: tighten rules for ACCESS ONCE Christian Borntraeger
@ 2015-01-15 8:58 ` Christian Borntraeger
2015-01-15 8:58 ` [PATCH 8/8] kernel: Fix sparse warning for ACCESS_ONCE Christian Borntraeger
2015-01-16 12:12 ` [PATCH 0/8] current ACCESS_ONCE patch queue Alexander Graf
8 siblings, 0 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm,
Guenter Roeck, Paul E. McKenney, Christian Borntraeger
From: Guenter Roeck <linux@roeck-us.net>
Commit a91ed664749c ("kernel: tighten rules for ACCESS ONCE") results in a
compile failure for sh builds with CONFIG_X2TLB enabled.
arch/sh/mm/gup.c: In function 'gup_get_pte':
arch/sh/mm/gup.c:20:2: error: invalid initializer
make[1]: *** [arch/sh/mm/gup.o] Error 1
Replace ACCESS_ONCE with READ_ONCE to fix the problem.
Fixes: a91ed664749c ("kernel: tighten rules for ACCESS ONCE")
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/sh/mm/gup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sh/mm/gup.c b/arch/sh/mm/gup.c
index 37458f3..e113bb4 100644
--- a/arch/sh/mm/gup.c
+++ b/arch/sh/mm/gup.c
@@ -17,7 +17,7 @@
static inline pte_t gup_get_pte(pte_t *ptep)
{
#ifndef CONFIG_X2TLB
- return ACCESS_ONCE(*ptep);
+ return READ_ONCE(*ptep);
#else
/*
* With get_user_pages_fast, we walk down the pagetables without
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 8/8] kernel: Fix sparse warning for ACCESS_ONCE
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
` (6 preceding siblings ...)
2015-01-15 8:58 ` [PATCH 7/8] next: sh: Fix compile error Christian Borntraeger
@ 2015-01-15 8:58 ` Christian Borntraeger
2015-01-16 12:12 ` [PATCH 0/8] current ACCESS_ONCE patch queue Alexander Graf
8 siblings, 0 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm,
Christian Borntraeger
Commit a91ed664749c ("kernel: tighten rules for ACCESS ONCE") results in
sparse warnings like "Using plain integer as NULL pointer" - Let's add a
type cast to the dummy assignment.
To avoid warnings lik "sparse: warning: cast to restricted __hc32" we also
use __force on that cast.
Fixes: a91ed664749c ("kernel: tighten rules for ACCESS ONCE")
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
include/linux/compiler.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 5e186bf..7bebf05 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -461,7 +461,7 @@ static __always_inline void __assign_once_size(volatile void *p, void *res, int
* If possible use READ_ONCE/ASSIGN_ONCE instead.
*/
#define __ACCESS_ONCE(x) ({ \
- __maybe_unused typeof(x) __var = 0; \
+ __maybe_unused typeof(x) __var = (__force typeof(x)) 0; \
(volatile typeof(x) *)&(x); })
#define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
--
1.9.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
2015-01-15 8:58 ` [PATCH 3/8] x86/xen/p2m: " Christian Borntraeger
@ 2015-01-15 9:26 ` Jürgen Groß
2015-01-15 10:43 ` [Xen-devel] " David Vrabel
1 sibling, 0 replies; 19+ messages in thread
From: Jürgen Groß @ 2015-01-15 9:26 UTC (permalink / raw)
To: Christian Borntraeger, linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm
On 01/15/2015 09:58 AM, Christian Borntraeger wrote:
> ACCESS_ONCE does not work reliably on non-scalar types. For
> example gcc 4.6 and 4.7 might remove the volatile tag for such
> accesses during the SRA (scalar replacement of aggregates) step
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
>
> Change the p2m code to replace ACCESS_ONCE with READ_ONCE.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
> ---
> arch/x86/xen/p2m.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
> index edbc7a6..cb71016 100644
> --- a/arch/x86/xen/p2m.c
> +++ b/arch/x86/xen/p2m.c
> @@ -554,7 +554,7 @@ static bool alloc_p2m(unsigned long pfn)
> mid_mfn = NULL;
> }
>
> - p2m_pfn = pte_pfn(ACCESS_ONCE(*ptep));
> + p2m_pfn = pte_pfn(READ_ONCE(*ptep));
> if (p2m_pfn == PFN_DOWN(__pa(p2m_identity)) ||
> p2m_pfn == PFN_DOWN(__pa(p2m_missing))) {
> /* p2m leaf page is missing */
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Xen-devel] [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
2015-01-15 8:58 ` [PATCH 3/8] x86/xen/p2m: " Christian Borntraeger
2015-01-15 9:26 ` Jürgen Groß
@ 2015-01-15 10:43 ` David Vrabel
2015-01-15 11:07 ` Christian Borntraeger
1 sibling, 1 reply; 19+ messages in thread
From: David Vrabel @ 2015-01-15 10:43 UTC (permalink / raw)
To: Christian Borntraeger, linux-kernel
Cc: linux-arch, kvm, x86, kvm-ppc, linux-mm, xen-devel, linuxppc-dev
On 15/01/15 08:58, Christian Borntraeger wrote:
> ACCESS_ONCE does not work reliably on non-scalar types. For
> example gcc 4.6 and 4.7 might remove the volatile tag for such
> accesses during the SRA (scalar replacement of aggregates) step
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
>
> Change the p2m code to replace ACCESS_ONCE with READ_ONCE.
Acked-by: David Vrabel <david.vrabel@citrix.com>
Let me know if you want me to merge this via the Xen tree.
David
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Xen-devel] [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
2015-01-15 10:43 ` [Xen-devel] " David Vrabel
@ 2015-01-15 11:07 ` Christian Borntraeger
0 siblings, 0 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 11:07 UTC (permalink / raw)
To: David Vrabel, linux-kernel
Cc: linux-arch, kvm, x86, kvm-ppc, linux-mm, xen-devel, linuxppc-dev
Am 15.01.2015 um 11:43 schrieb David Vrabel:
> On 15/01/15 08:58, Christian Borntraeger wrote:
>> ACCESS_ONCE does not work reliably on non-scalar types. For
>> example gcc 4.6 and 4.7 might remove the volatile tag for such
>> accesses during the SRA (scalar replacement of aggregates) step
>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
>>
>> Change the p2m code to replace ACCESS_ONCE with READ_ONCE.
>
> Acked-by: David Vrabel <david.vrabel@citrix.com>
Thanks
> Let me know if you want me to merge this via the Xen tree.
With your Acked-by, I can easily carry this in my tree. We can
then ensure that this patch is merged before the ACCESS_ONCE is
made stricter - making bisecting easier.
Christian
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE
2015-01-15 8:58 ` [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE Christian Borntraeger
@ 2015-01-15 19:38 ` Oleg Nesterov
2015-01-15 19:51 ` Christian Borntraeger
0 siblings, 1 reply; 19+ messages in thread
From: Oleg Nesterov @ 2015-01-15 19:38 UTC (permalink / raw)
To: Christian Borntraeger
Cc: linux-kernel, linux-arch, kvm, kvm-ppc, linuxppc-dev, x86,
xen-devel, linux-mm
On 01/15, Christian Borntraeger wrote:
>
> --- a/arch/x86/include/asm/spinlock.h
> +++ b/arch/x86/include/asm/spinlock.h
> @@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
> __ticket_t head = ACCESS_ONCE(lock->tickets.head);
>
> for (;;) {
> - struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
> + struct __raw_tickets tmp = READ_ONCE(lock->tickets);
Agreed, but what about another ACCESS_ONCE() above?
Oleg.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE
2015-01-15 19:38 ` Oleg Nesterov
@ 2015-01-15 19:51 ` Christian Borntraeger
2015-01-15 20:01 ` Oleg Nesterov
0 siblings, 1 reply; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 19:51 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-kernel, linux-arch, kvm, kvm-ppc, linuxppc-dev, x86,
xen-devel, linux-mm
Am 15.01.2015 um 20:38 schrieb Oleg Nesterov:
> On 01/15, Christian Borntraeger wrote:
>>
>> --- a/arch/x86/include/asm/spinlock.h
>> +++ b/arch/x86/include/asm/spinlock.h
>> @@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
>> __ticket_t head = ACCESS_ONCE(lock->tickets.head);
>>
>> for (;;) {
>> - struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
>> + struct __raw_tickets tmp = READ_ONCE(lock->tickets);
>
> Agreed, but what about another ACCESS_ONCE() above?
>
> Oleg.
>
tickets.head is a scalar type, so ACCESS_ONCE does work fine with gcc 4.6/4.7.
My goal was to convert all accesses on non-scalar types as until
"kernel: tighten rules for ACCESS ONCE" is merged because anything else would be
a Whac-a-mole like adventure (I learned that during the last round in next: all
conversions in this series fix up changes made during this merge window)
We probably going to do a bigger bunch of bulk conversion later on when
"kernel: tighten rules for ACCESS ONCE" prevents new problems.
Makes sense?
Christian
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE
2015-01-15 19:51 ` Christian Borntraeger
@ 2015-01-15 20:01 ` Oleg Nesterov
2015-01-15 21:00 ` Christian Borntraeger
0 siblings, 1 reply; 19+ messages in thread
From: Oleg Nesterov @ 2015-01-15 20:01 UTC (permalink / raw)
To: Christian Borntraeger
Cc: linux-kernel, linux-arch, kvm, kvm-ppc, linuxppc-dev, x86,
xen-devel, linux-mm
On 01/15, Christian Borntraeger wrote:
>
> Am 15.01.2015 um 20:38 schrieb Oleg Nesterov:
> > On 01/15, Christian Borntraeger wrote:
> >>
> >> --- a/arch/x86/include/asm/spinlock.h
> >> +++ b/arch/x86/include/asm/spinlock.h
> >> @@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
> >> __ticket_t head = ACCESS_ONCE(lock->tickets.head);
> >>
> >> for (;;) {
> >> - struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
> >> + struct __raw_tickets tmp = READ_ONCE(lock->tickets);
> >
> > Agreed, but what about another ACCESS_ONCE() above?
> >
> > Oleg.
>
> tickets.head is a scalar type, so ACCESS_ONCE does work fine with gcc 4.6/4.7.
> My goal was to convert all accesses on non-scalar types
I understand, but READ_ONCE(lock->tickets.head) looks better anyway and
arch_spin_lock() already use READ_ONCE() for this.
So why we should keep the last ACCESS_ONCE() in spinlock.h ? Just to make
another cosmetic cleanup which touches the same function later?
Oleg.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE
2015-01-15 20:01 ` Oleg Nesterov
@ 2015-01-15 21:00 ` Christian Borntraeger
0 siblings, 0 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-15 21:00 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-kernel, linux-arch, kvm, kvm-ppc, linuxppc-dev, x86,
xen-devel, linux-mm
Am 15.01.2015 um 21:01 schrieb Oleg Nesterov:
> On 01/15, Christian Borntraeger wrote:
>>
>> Am 15.01.2015 um 20:38 schrieb Oleg Nesterov:
>>> On 01/15, Christian Borntraeger wrote:
>>>>
>>>> --- a/arch/x86/include/asm/spinlock.h
>>>> +++ b/arch/x86/include/asm/spinlock.h
>>>> @@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
>>>> __ticket_t head = ACCESS_ONCE(lock->tickets.head);
>>>>
>>>> for (;;) {
>>>> - struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
>>>> + struct __raw_tickets tmp = READ_ONCE(lock->tickets);
>>>
>>> Agreed, but what about another ACCESS_ONCE() above?
>>>
>>> Oleg.
>>
>> tickets.head is a scalar type, so ACCESS_ONCE does work fine with gcc 4.6/4.7.
>> My goal was to convert all accesses on non-scalar types
>
> I understand, but READ_ONCE(lock->tickets.head) looks better anyway and
> arch_spin_lock() already use READ_ONCE() for this.
>
> So why we should keep the last ACCESS_ONCE() in spinlock.h ? Just to make
> another cosmetic cleanup which touches the same function later?
OK, I will change that one as well.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE
2015-01-15 8:58 ` [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE Christian Borntraeger
@ 2015-01-15 23:09 ` Michael Ellerman
2015-01-16 9:43 ` Christian Borntraeger
0 siblings, 1 reply; 19+ messages in thread
From: Michael Ellerman @ 2015-01-15 23:09 UTC (permalink / raw)
To: Christian Borntraeger
Cc: linux-kernel, linux-arch, kvm, kvm-ppc, linuxppc-dev, x86,
xen-devel, linux-mm
On Thu, 2015-01-15 at 09:58 +0100, Christian Borntraeger wrote:
> ACCESS_ONCE does not work reliably on non-scalar types. For
> example gcc 4.6 and 4.7 might remove the volatile tag for such
> accesses during the SRA (scalar replacement of aggregates) step
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
>
> Change the ppc/kvm code to replace ACCESS_ONCE with READ_ONCE.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv_rm_xics.c | 8 ++++----
> arch/powerpc/kvm/book3s_xics.c | 16 ++++++++--------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c b/arch/powerpc/kvm/book3s_hv_rm_xics.c
> index 7b066f6..7c22997 100644
> --- a/arch/powerpc/kvm/book3s_hv_rm_xics.c
> +++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c
> @@ -152,7 +152,7 @@ static void icp_rm_down_cppr(struct kvmppc_xics *xics, struct kvmppc_icp *icp,
> * in virtual mode.
> */
> do {
> - old_state = new_state = ACCESS_ONCE(icp->state);
> + old_state = new_state = READ_ONCE(icp->state);
These are all icp->state.
Which is a union, but it's only the size of unsigned long. So in practice there
shouldn't be a bug here right?
cheers
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE
2015-01-15 23:09 ` Michael Ellerman
@ 2015-01-16 9:43 ` Christian Borntraeger
0 siblings, 0 replies; 19+ messages in thread
From: Christian Borntraeger @ 2015-01-16 9:43 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-kernel, linux-arch, kvm, kvm-ppc, linuxppc-dev, x86,
xen-devel, linux-mm
Am 16.01.2015 um 00:09 schrieb Michael Ellerman:
> On Thu, 2015-01-15 at 09:58 +0100, Christian Borntraeger wrote:
>> ACCESS_ONCE does not work reliably on non-scalar types. For
>> example gcc 4.6 and 4.7 might remove the volatile tag for such
>> accesses during the SRA (scalar replacement of aggregates) step
>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
>>
>> Change the ppc/kvm code to replace ACCESS_ONCE with READ_ONCE.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>> arch/powerpc/kvm/book3s_hv_rm_xics.c | 8 ++++----
>> arch/powerpc/kvm/book3s_xics.c | 16 ++++++++--------
>> 2 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c b/arch/powerpc/kvm/book3s_hv_rm_xics.c
>> index 7b066f6..7c22997 100644
>> --- a/arch/powerpc/kvm/book3s_hv_rm_xics.c
>> +++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c
>> @@ -152,7 +152,7 @@ static void icp_rm_down_cppr(struct kvmppc_xics *xics, struct kvmppc_icp *icp,
>> * in virtual mode.
>> */
>> do {
>> - old_state = new_state = ACCESS_ONCE(icp->state);
>> + old_state = new_state = READ_ONCE(icp->state);
>
> These are all icp->state.
>
> Which is a union, but it's only the size of unsigned long. So in practice there
> shouldn't be a bug here right?
This bug was that gcc lost the volatile tag when propagating aggregates to scalar types.
So in theory a union could be affected. See the original problem
( http://marc.info/?i=54611D86.4040306%40de.ibm.com )
which happened on
union ipte_control {
unsigned long val;
struct {
unsigned long k : 1;
unsigned long kh : 31;
unsigned long kg : 32;
};
};
Christian
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/8] current ACCESS_ONCE patch queue
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
` (7 preceding siblings ...)
2015-01-15 8:58 ` [PATCH 8/8] kernel: Fix sparse warning for ACCESS_ONCE Christian Borntraeger
@ 2015-01-16 12:12 ` Alexander Graf
8 siblings, 0 replies; 19+ messages in thread
From: Alexander Graf @ 2015-01-16 12:12 UTC (permalink / raw)
To: Christian Borntraeger, linux-kernel
Cc: linux-arch, kvm, kvm-ppc, linuxppc-dev, x86, xen-devel, linux-mm
On 15.01.15 09:58, Christian Borntraeger wrote:
> Folks,
>
> fyi, this is my current patch queue for the next merge window. It
> does contain a patch that will disallow ACCESS_ONCE on non-scalar
> types.
>
> The tree is part of linux-next and can be found at
> git://git.kernel.org/pub/scm/linux/kernel/git/borntraeger/linux.git linux-next
KVM PPC bits are:
Acked-by: Alexander Graf <agraf@suse.de>
Alex
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-01-16 12:12 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-15 8:58 [PATCH 0/8] current ACCESS_ONCE patch queue Christian Borntraeger
2015-01-15 8:58 ` [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE Christian Borntraeger
2015-01-15 23:09 ` Michael Ellerman
2015-01-16 9:43 ` Christian Borntraeger
2015-01-15 8:58 ` [PATCH 2/8] ppc/hugetlbfs: " Christian Borntraeger
2015-01-15 8:58 ` [PATCH 3/8] x86/xen/p2m: " Christian Borntraeger
2015-01-15 9:26 ` Jürgen Groß
2015-01-15 10:43 ` [Xen-devel] " David Vrabel
2015-01-15 11:07 ` Christian Borntraeger
2015-01-15 8:58 ` [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE Christian Borntraeger
2015-01-15 19:38 ` Oleg Nesterov
2015-01-15 19:51 ` Christian Borntraeger
2015-01-15 20:01 ` Oleg Nesterov
2015-01-15 21:00 ` Christian Borntraeger
2015-01-15 8:58 ` [PATCH 5/8] mm/gup: Replace ACCESS_ONCE with READ_ONCE Christian Borntraeger
2015-01-15 8:58 ` [PATCH 6/8] kernel: tighten rules for ACCESS ONCE Christian Borntraeger
2015-01-15 8:58 ` [PATCH 7/8] next: sh: Fix compile error Christian Borntraeger
2015-01-15 8:58 ` [PATCH 8/8] kernel: Fix sparse warning for ACCESS_ONCE Christian Borntraeger
2015-01-16 12:12 ` [PATCH 0/8] current ACCESS_ONCE patch queue Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).