Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1608382 > unrolled thread
| Started by | Dmitry Vyukov <dvyukov@google.com> |
|---|---|
| First post | 2017-03-24 13:50 +0100 |
| Last post | 2017-03-27 15:50 +0200 |
| Articles | 17 on this page of 37 — 5 participants |
Back to article view | Back to linux.kernel
re: locking/atomic: Introduce atomic_try_cmpxchg() Dmitry Vyukov <dvyukov@google.com> - 2017-03-24 13:50 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Dmitry Vyukov <dvyukov@google.com> - 2017-03-24 15:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 15:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 17:50 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Andy Lutomirski <luto@amacapital.net> - 2017-03-24 18:00 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 18:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Dmitry Vyukov <dvyukov@google.com> - 2017-03-24 19:00 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 19:20 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 19:20 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Andy Lutomirski <luto@kernel.org> - 2017-03-24 20:20 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-24 20:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Andy Lutomirski <luto@amacapital.net> - 2017-03-24 20:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 21:20 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 21:20 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Andy Lutomirski <luto@amacapital.net> - 2017-03-24 21:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Dmitry Vyukov <dvyukov@google.com> - 2017-03-24 19:20 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 19:10 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 19:20 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Andy Lutomirski <luto@amacapital.net> - 2017-03-24 19:50 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-24 20:20 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 22:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-25 09:00 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-25 19:10 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-25 19:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-25 19:40 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-25 22:20 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-25 23:10 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-27 12:00 +0200
Re: locking/atomic: Introduce atomic_try_cmpxchg() Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-25 19:40 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 21:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Andy Lutomirski <luto@kernel.org> - 2017-03-24 21:30 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 22:10 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-24 20:10 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-24 21:50 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-24 22:00 +0100
Re: locking/atomic: Introduce atomic_try_cmpxchg() Peter Zijlstra <peterz@infradead.org> - 2017-03-27 14:20 +0200
Re: locking/atomic: Introduce atomic_try_cmpxchg() Dmitry Vyukov <dvyukov@google.com> - 2017-03-27 15:50 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-03-24 22:30 +0100 |
| Message-ID | <toEWu-7xX-11@gated-at.bofh.it> |
| In reply to | #1608902 |
On Fri, Mar 24, 2017 at 12:17:28PM -0700, Linus Torvalds wrote: > On Fri, Mar 24, 2017 at 11:45 AM, Andy Lutomirski <luto@amacapital.net> wrote: > > > > Is there some hack like if __builtin_is_unescaped(*val) *val = old; > > that would work? > > See my recent email suggesting a completely different interface, which > avoids this problem. > > My interface generates: > > 0000000000000000 <T_refcount_inc>: > 0: 8b 07 mov (%rdi),%eax > 2: 83 f8 ff cmp $0xffffffff,%eax > 5: 74 12 je 19 <T_refcount_inc+0x19> > 7: 85 c0 test %eax,%eax > 9: 74 0a je 15 <T_refcount_inc+0x15> > b: 8d 50 01 lea 0x1(%rax),%edx > e: f0 0f b1 17 lock cmpxchg %edx,(%rdi) > 12: 75 ee jne 2 <T_refcount_inc+0x2> > 14: c3 retq > 15: 31 c0 xor %eax,%eax > 17: 0f 0b ud2 > 19: c3 retq > > for PeterZ's test-case, which seems optimal. Right; now my GCC emits more or less the same code (its a slightly different compiler and instead of 12: jne, it does: 12 je ; 14: jmp 2. But maybe that's the likely() you added later. Also, see how at 7 we test if eax is 0 and then at 9 jump to 15 where we make eax 0. Pretty daft code-gen. In any case, you lost one branch into ud2; your success: return, should be success: if (new == UINT_MAX), such that when we newly saturate the count we also raise an exception. With that, the code is still larger than it used to be. I'll have a play around. I do like this interface better, but getting GCC to generate sensible code seems 'interesting'. I'll try and redo the patches that landed in tip and see what it does for total vmlinux size somewhere tomorrow.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-03-25 09:00 +0100 |
| Message-ID | <toOMa-67F-15@gated-at.bofh.it> |
| In reply to | #1608983 |
On Fri, Mar 24, 2017 at 10:23:29PM +0100, Peter Zijlstra wrote:
> I'll try and redo the patches that landed in tip and see what it does
> for total vmlinux size somewhere tomorrow.
text data bss dec hex filename
10726413 4540256 843776 16110445 f5d36d defconfig-build/vmlinux.pre
10730509 4540256 843776 16114541 f5e36d defconfig-build/vmlinux.post
:-(
---
arch/x86/include/asm/atomic.h | 18 ++++++-------
arch/x86/include/asm/atomic64_64.h | 24 ++++++++++--------
arch/x86/include/asm/cmpxchg.h | 50 ++++++++++++++++++++----------------
include/linux/atomic.h | 52 +++++++++++++++++++++++++-------------
lib/refcount.c | 35 ++++++++++++++-----------
5 files changed, 104 insertions(+), 75 deletions(-)
diff --git a/arch/x86/include/asm/atomic.h b/arch/x86/include/asm/atomic.h
index caa5798..c80d4914 100644
--- a/arch/x86/include/asm/atomic.h
+++ b/arch/x86/include/asm/atomic.h
@@ -186,11 +186,8 @@ static __always_inline int atomic_cmpxchg(atomic_t *v, int old, int new)
return cmpxchg(&v->counter, old, new);
}
-#define atomic_try_cmpxchg atomic_try_cmpxchg
-static __always_inline bool atomic_try_cmpxchg(atomic_t *v, int *old, int new)
-{
- return try_cmpxchg(&v->counter, old, new);
-}
+#define atomic_try_cmpxchg(_ptr, _old, _new, _label) \
+ try_cmpxchg(&(_ptr)->counter, _old, _new, _label)
static inline int atomic_xchg(atomic_t *v, int new)
{
@@ -210,8 +207,9 @@ static inline void atomic_##op(int i, atomic_t *v) \
static inline int atomic_fetch_##op(int i, atomic_t *v) \
{ \
int val = atomic_read(v); \
- do { \
- } while (!atomic_try_cmpxchg(v, &val, val c_op i)); \
+ for (;;) \
+ val = atomic_try_cmpxchg(v, val, val c_op i, success); \
+success: \
return val; \
}
@@ -239,10 +237,12 @@ ATOMIC_OPS(xor, ^)
static __always_inline int __atomic_add_unless(atomic_t *v, int a, int u)
{
int c = atomic_read(v);
- do {
+ for (;;) {
if (unlikely(c == u))
break;
- } while (!atomic_try_cmpxchg(v, &c, c + a));
+ c = atomic_try_cmpxchg(v, c, c + a, success);
+ }
+success:
return c;
}
diff --git a/arch/x86/include/asm/atomic64_64.h b/arch/x86/include/asm/atomic64_64.h
index 6189a43..489c3e2 100644
--- a/arch/x86/include/asm/atomic64_64.h
+++ b/arch/x86/include/asm/atomic64_64.h
@@ -176,11 +176,8 @@ static inline long atomic64_cmpxchg(atomic64_t *v, long old, long new)
return cmpxchg(&v->counter, old, new);
}
-#define atomic64_try_cmpxchg atomic64_try_cmpxchg
-static __always_inline bool atomic64_try_cmpxchg(atomic64_t *v, long *old, long new)
-{
- return try_cmpxchg(&v->counter, old, new);
-}
+#define atomic64_try_cmpxchg(_ptr, _old, _new, _label) \
+ try_cmpxchg(&(_ptr)->counter, _old, _new, _label)
static inline long atomic64_xchg(atomic64_t *v, long new)
{
@@ -199,10 +196,12 @@ static inline long atomic64_xchg(atomic64_t *v, long new)
static inline bool atomic64_add_unless(atomic64_t *v, long a, long u)
{
long c = atomic64_read(v);
- do {
+ for (;;) {
if (unlikely(c == u))
return false;
- } while (!atomic64_try_cmpxchg(v, &c, c + a));
+ c = atomic64_try_cmpxchg(v, c, c + a, success);
+ }
+success:
return true;
}
@@ -218,11 +217,13 @@ static inline bool atomic64_add_unless(atomic64_t *v, long a, long u)
static inline long atomic64_dec_if_positive(atomic64_t *v)
{
long dec, c = atomic64_read(v);
- do {
+ for (;;) {
dec = c - 1;
if (unlikely(dec < 0))
break;
- } while (!atomic64_try_cmpxchg(v, &c, dec));
+ c = atomic64_try_cmpxchg(v, c, dec, success);
+ }
+success:
return dec;
}
@@ -239,8 +240,9 @@ static inline void atomic64_##op(long i, atomic64_t *v) \
static inline long atomic64_fetch_##op(long i, atomic64_t *v) \
{ \
long val = atomic64_read(v); \
- do { \
- } while (!atomic64_try_cmpxchg(v, &val, val c_op i)); \
+ for (;;) \
+ val = atomic64_try_cmpxchg(v, val, val c_op i, success);\
+success: \
return val; \
}
diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
index fb961db..e6b8a8f 100644
--- a/arch/x86/include/asm/cmpxchg.h
+++ b/arch/x86/include/asm/cmpxchg.h
@@ -154,22 +154,24 @@ extern void __add_wrong_size(void)
__cmpxchg_local(ptr, old, new, sizeof(*(ptr)))
-#define __raw_try_cmpxchg(_ptr, _pold, _new, size, lock) \
+#define __raw_try_cmpxchg(_ptr, _old, _new, success_label, size, lock) \
({ \
- bool success; \
- __typeof__(_ptr) _old = (_pold); \
- __typeof__(*(_ptr)) __old = *_old; \
+ __typeof__(*(_ptr)) __old = (_old); \
__typeof__(*(_ptr)) __new = (_new); \
+ __typeof__(*(_ptr)) __ret; \
+ bool __success; \
+ \
switch (size) { \
case __X86_CASE_B: \
{ \
volatile u8 *__ptr = (volatile u8 *)(_ptr); \
asm volatile(lock "cmpxchgb %[new], %[ptr]" \
CC_SET(z) \
- : CC_OUT(z) (success), \
+ : CC_OUT(z) (__success), \
[ptr] "+m" (*__ptr), \
- [old] "+a" (__old) \
- : [new] "q" (__new) \
+ [old] "=a" (__ret) \
+ : [new] "q" (__new), \
+ "2" (__old) \
: "memory"); \
break; \
} \
@@ -178,10 +180,11 @@ extern void __add_wrong_size(void)
volatile u16 *__ptr = (volatile u16 *)(_ptr); \
asm volatile(lock "cmpxchgw %[new], %[ptr]" \
CC_SET(z) \
- : CC_OUT(z) (success), \
+ : CC_OUT(z) (__success), \
[ptr] "+m" (*__ptr), \
- [old] "+a" (__old) \
- : [new] "r" (__new) \
+ [old] "=a" (__ret) \
+ : [new] "r" (__new), \
+ "2" (__old) \
: "memory"); \
break; \
} \
@@ -190,10 +193,11 @@ extern void __add_wrong_size(void)
volatile u32 *__ptr = (volatile u32 *)(_ptr); \
asm volatile(lock "cmpxchgl %[new], %[ptr]" \
CC_SET(z) \
- : CC_OUT(z) (success), \
+ : CC_OUT(z) (__success), \
[ptr] "+m" (*__ptr), \
- [old] "+a" (__old) \
- : [new] "r" (__new) \
+ [old] "=a" (__ret) \
+ : [new] "r" (__new), \
+ "2" (__old) \
: "memory"); \
break; \
} \
@@ -202,25 +206,27 @@ extern void __add_wrong_size(void)
volatile u64 *__ptr = (volatile u64 *)(_ptr); \
asm volatile(lock "cmpxchgq %[new], %[ptr]" \
CC_SET(z) \
- : CC_OUT(z) (success), \
+ : CC_OUT(z) (__success), \
[ptr] "+m" (*__ptr), \
- [old] "+a" (__old) \
- : [new] "r" (__new) \
+ [old] "=a" (__ret) \
+ : [new] "r" (__new), \
+ "2" (__old) \
: "memory"); \
break; \
} \
default: \
__cmpxchg_wrong_size(); \
} \
- *_old = __old; \
- success; \
+ \
+ if (likely(__success)) goto success_label; \
+ __ret; \
})
-#define __try_cmpxchg(ptr, pold, new, size) \
- __raw_try_cmpxchg((ptr), (pold), (new), (size), LOCK_PREFIX)
+#define __try_cmpxchg(ptr, pold, new, success_label, size) \
+ __raw_try_cmpxchg((ptr), (pold), (new), success_label, (size), LOCK_PREFIX)
-#define try_cmpxchg(ptr, pold, new) \
- __try_cmpxchg((ptr), (pold), (new), sizeof(*(ptr)))
+#define try_cmpxchg(ptr, pold, new, success_label) \
+ __try_cmpxchg((ptr), (pold), (new), success_label, sizeof(*(ptr)))
/*
* xadd() adds "inc" to "*ptr" and atomically returns the previous
diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index aae5953..13a6eac 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -425,18 +425,26 @@
#ifndef atomic_try_cmpxchg
-#define __atomic_try_cmpxchg(type, _p, _po, _n) \
+#define __atomic_try_cmpxchg(type, _p, _o, _n, _label) \
({ \
- typeof(_po) __po = (_po); \
- typeof(*(_po)) __o = *__po; \
- *__po = atomic_cmpxchg##type((_p), __o, (_n)); \
- (*__po == __o); \
+ typeof(*(_p)) __r; \
+ typeof(*(_p)) __o = (_o); \
+ __r = atomic_cmpxchg##type((_p), __o, (_n)); \
+ if (__r == __o) goto _label; \
+ __r; \
})
-#define atomic_try_cmpxchg(_p, _po, _n) __atomic_try_cmpxchg(, _p, _po, _n)
-#define atomic_try_cmpxchg_relaxed(_p, _po, _n) __atomic_try_cmpxchg(_relaxed, _p, _po, _n)
-#define atomic_try_cmpxchg_acquire(_p, _po, _n) __atomic_try_cmpxchg(_acquire, _p, _po, _n)
-#define atomic_try_cmpxchg_release(_p, _po, _n) __atomic_try_cmpxchg(_release, _p, _po, _n)
+#define atomic_try_cmpxchg(_p, _o, _n, _l) \
+ __atomic_try_cmpxchg(, _p, _o, _n, _l)
+
+#define atomic_try_cmpxchg_relaxed(_p, _o, _n, _l) \
+ __atomic_try_cmpxchg(_relaxed, _p, _o, _n, _l)
+
+#define atomic_try_cmpxchg_acquire(_p, _o, _n, _l) \
+ __atomic_try_cmpxchg(_acquire, _p, _o, _n, _l)
+
+#define atomic_try_cmpxchg_release(_p, _o, _n, _l) \
+ __atomic_try_cmpxchg(_release, _p, _o, _n, _l)
#else /* atomic_try_cmpxchg */
#define atomic_try_cmpxchg_relaxed atomic_try_cmpxchg
@@ -1019,18 +1027,26 @@ static inline int atomic_dec_if_positive(atomic_t *v)
#ifndef atomic64_try_cmpxchg
-#define __atomic64_try_cmpxchg(type, _p, _po, _n) \
+#define __atomic64_try_cmpxchg(type, _p, _o, _n, _label) \
({ \
- typeof(_po) __po = (_po); \
- typeof(*(_po)) __o = *__po; \
- *__po = atomic64_cmpxchg##type((_p), __o, (_n)); \
- (*__po == __o); \
+ typeof(*(_p)) __r; \
+ typeof(*(_p)) __o = (_o); \
+ __r = atomic64_cmpxchg##type((_p), __o, (_n)); \
+ if (__r == __o) goto _label; \
+ __r; \
})
-#define atomic64_try_cmpxchg(_p, _po, _n) __atomic64_try_cmpxchg(, _p, _po, _n)
-#define atomic64_try_cmpxchg_relaxed(_p, _po, _n) __atomic64_try_cmpxchg(_relaxed, _p, _po, _n)
-#define atomic64_try_cmpxchg_acquire(_p, _po, _n) __atomic64_try_cmpxchg(_acquire, _p, _po, _n)
-#define atomic64_try_cmpxchg_release(_p, _po, _n) __atomic64_try_cmpxchg(_release, _p, _po, _n)
+#define atomic64_try_cmpxchg(_p, _o, _n, _l) \
+ __atomic64_try_cmpxchg(, _p, _o, _n, _l)
+
+#define atomic64_try_cmpxchg_relaxed(_p, _o, _n, _l) \
+ __atomic64_try_cmpxchg(_relaxed, _p, _o, _n, _l)
+
+#define atomic64_try_cmpxchg_acquire(_p, _o, _n, _l) \
+ __atomic64_try_cmpxchg(_acquire, _p, _o, _n, _l)
+
+#define atomic64_try_cmpxchg_release(_p, _o, _n, _l) \
+ __atomic64_try_cmpxchg(_release, _p, _o, _n, _l)
#else /* atomic64_try_cmpxchg */
#define atomic64_try_cmpxchg_relaxed atomic64_try_cmpxchg
diff --git a/lib/refcount.c b/lib/refcount.c
index f42124c..18b8926 100644
--- a/lib/refcount.c
+++ b/lib/refcount.c
@@ -59,7 +59,7 @@ bool refcount_add_not_zero(unsigned int i, refcount_t *r)
{
unsigned int new, val = atomic_read(&r->refs);
- do {
+ for (;;) {
if (!val)
return false;
@@ -70,8 +70,9 @@ bool refcount_add_not_zero(unsigned int i, refcount_t *r)
if (new < val)
new = UINT_MAX;
- } while (!atomic_try_cmpxchg_relaxed(&r->refs, &val, new));
-
+ val = atomic_try_cmpxchg_relaxed(&r->refs, val, new, success);
+ }
+success:
WARN_ONCE(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
return true;
@@ -116,7 +117,7 @@ bool refcount_inc_not_zero(refcount_t *r)
{
unsigned int new, val = atomic_read(&r->refs);
- do {
+ for (;;) {
new = val + 1;
if (!val)
@@ -125,8 +126,9 @@ bool refcount_inc_not_zero(refcount_t *r)
if (unlikely(!new))
return true;
- } while (!atomic_try_cmpxchg_relaxed(&r->refs, &val, new));
-
+ val = atomic_try_cmpxchg_relaxed(&r->refs, val, new, success);
+ }
+success:
WARN_ONCE(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
return true;
@@ -175,7 +177,7 @@ bool refcount_sub_and_test(unsigned int i, refcount_t *r)
{
unsigned int new, val = atomic_read(&r->refs);
- do {
+ for (;;) {
if (unlikely(val == UINT_MAX))
return false;
@@ -185,8 +187,9 @@ bool refcount_sub_and_test(unsigned int i, refcount_t *r)
return false;
}
- } while (!atomic_try_cmpxchg_release(&r->refs, &val, new));
-
+ val = atomic_try_cmpxchg_release(&r->refs, val, new, success);
+ }
+success:
return !new;
}
EXPORT_SYMBOL_GPL(refcount_sub_and_test);
@@ -244,9 +247,10 @@ EXPORT_SYMBOL_GPL(refcount_dec);
*/
bool refcount_dec_if_one(refcount_t *r)
{
- int val = 1;
-
- return atomic_try_cmpxchg_release(&r->refs, &val, 0);
+ atomic_try_cmpxchg_release(&r->refs, 1, 0, success);
+ return false;
+success:
+ return true;
}
EXPORT_SYMBOL_GPL(refcount_dec_if_one);
@@ -265,7 +269,7 @@ bool refcount_dec_not_one(refcount_t *r)
{
unsigned int new, val = atomic_read(&r->refs);
- do {
+ for (;;) {
if (unlikely(val == UINT_MAX))
return true;
@@ -278,8 +282,9 @@ bool refcount_dec_not_one(refcount_t *r)
return true;
}
- } while (!atomic_try_cmpxchg_release(&r->refs, &val, new));
-
+ val = atomic_try_cmpxchg_release(&r->refs, val, new, success);
+ }
+success:
return true;
}
EXPORT_SYMBOL_GPL(refcount_dec_not_one);
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-03-25 19:10 +0100 |
| Message-ID | <toYit-4wW-1@gated-at.bofh.it> |
| In reply to | #1609119 |
On Sat, Mar 25, 2017 at 12:51 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 24, 2017 at 10:23:29PM +0100, Peter Zijlstra wrote:
>
>> I'll try and redo the patches that landed in tip and see what it does
>> for total vmlinux size somewhere tomorrow.
>
> text data bss dec hex filename
> 10726413 4540256 843776 16110445 f5d36d defconfig-build/vmlinux.pre
> 10730509 4540256 843776 16114541 f5e36d defconfig-build/vmlinux.post
>
> :-(
Hmm. But you are comparing against the *broken* version that did the
unconditional store of the result.
You should at least compare against the fixed version with the
conditional store. That's the one that was hard to get good code
generation from, wasn't it?
Linus
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-03-25 19:30 +0100 |
| Message-ID | <toYBQ-4DA-17@gated-at.bofh.it> |
| In reply to | #1609200 |
On Sat, Mar 25, 2017 at 11:00:44AM -0700, Linus Torvalds wrote:
> On Sat, Mar 25, 2017 at 12:51 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Fri, Mar 24, 2017 at 10:23:29PM +0100, Peter Zijlstra wrote:
> >
> >> I'll try and redo the patches that landed in tip and see what it does
> >> for total vmlinux size somewhere tomorrow.
> >
> > text data bss dec hex filename
> > 10726413 4540256 843776 16110445 f5d36d defconfig-build/vmlinux.pre
> > 10730509 4540256 843776 16114541 f5e36d defconfig-build/vmlinux.post
10730445 4540256 843776 16114477 f5e32d defconfig-build/vmlinux
> >
> > :-(
>
> Hmm. But you are comparing against the *broken* version that did the
> unconditional store of the result.
Well, only broken if not used on stack local variables, but yes.
> You should at least compare against the fixed version with the
> conditional store. That's the one that was hard to get good code
> generation from, wasn't it?
Added above, a few bytes smaller than the shiny new one actually.
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-03-25 19:40 +0100 |
| Message-ID | <toYLv-4H9-1@gated-at.bofh.it> |
| In reply to | #1609208 |
On Sat, Mar 25, 2017 at 11:28 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Hmm. Sad. The label approach looked like it would match the semantics
> of cmpxchg perfectly, but it's not as optimal as it superficially
> would have seemed.
Oh, I just noticed that at least your other one didn't mark "success"
as being likely.
That changed code generation a lot for me for the loops, where gcc
would assume that the loop was likely to be taken, which in turn means
that gcc lays out the loop with a backwards branch. Which is denser,
but also likely slower, since most x86 chips predict backwards
branches taken.
So that might be one difference. Although the size differences in the
last case are so small that it might also just be random noise.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-03-25 22:20 +0100 |
| Message-ID | <tp1gm-6A9-3@gated-at.bofh.it> |
| In reply to | #1609217 |
On Sat, Mar 25, 2017 at 11:34:32AM -0700, Linus Torvalds wrote: > On Sat, Mar 25, 2017 at 11:28 AM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > Hmm. Sad. The label approach looked like it would match the semantics > > of cmpxchg perfectly, but it's not as optimal as it superficially > > would have seemed. > > Oh, I just noticed that at least your other one didn't mark "success" > as being likely. 10730509 4540256 843776 16114541 f5e36d defconfig-build/vmlinux --- arch/x86/include/asm/cmpxchg.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h index fb961db..d347abc 100644 --- a/arch/x86/include/asm/cmpxchg.h +++ b/arch/x86/include/asm/cmpxchg.h @@ -212,8 +212,9 @@ extern void __add_wrong_size(void) default: \ __cmpxchg_wrong_size(); \ } \ + if (unlikely(!success)) \ *_old = __old; \ - success; \ + likely(success); \ }) #define __try_cmpxchg(ptr, pold, new, size) \
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-03-25 23:10 +0100 |
| Message-ID | <tp22K-77F-11@gated-at.bofh.it> |
| In reply to | #1609233 |
On Sat, Mar 25, 2017 at 2:13 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Sat, Mar 25, 2017 at 11:34:32AM -0700, Linus Torvalds wrote:
>>
>> Oh, I just noticed that at least your other one didn't mark "success"
>> as being likely.
>
> 10730509 4540256 843776 16114541 f5e36d defconfig-build/vmlinux
Ok, that seems to be the exact same size as with the patch using the
"goto label" approach. So maybe the code generation is the same now.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-03-27 12:00 +0200 |
| Message-ID | <tpzBp-66r-27@gated-at.bofh.it> |
| In reply to | #1609248 |
On Sat, Mar 25, 2017 at 03:08:10PM -0700, Linus Torvalds wrote: > On Sat, Mar 25, 2017 at 2:13 PM, Peter Zijlstra <peterz@infradead.org> wrote: > > On Sat, Mar 25, 2017 at 11:34:32AM -0700, Linus Torvalds wrote: > >> > >> Oh, I just noticed that at least your other one didn't mark "success" > >> as being likely. > > > > 10730509 4540256 843776 16114541 f5e36d defconfig-build/vmlinux > > Ok, that seems to be the exact same size as with the patch using the > "goto label" approach. So maybe the code generation is the same now. OK, so I went and build myself a GCC-7 compiler and constructed the below table. From this I would propose we do the "try_cmpxchg + if" thing, also below. Because, while the interface is icky, it is what C11 does for this construct. GCC-6.3.0: 10735757 (cmpxchg) 10726413 (try_cmpxchg) 10730701 (try_cmpxchg + likely) 10730509 (try_cmpxchg + if) 10730445 (try_cmpxchg-linus) GCC-7 (20170327): 10709514 (cmpxchg) 10704266 (try_cmpxchg) 10704458 (try_cmpxchg + likely) 10704266 (try_cmpxchg + if) 10704394 (try_cmpxchg-linus) --- arch/x86/include/asm/cmpxchg.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h index fb961db..d90296d 100644 --- a/arch/x86/include/asm/cmpxchg.h +++ b/arch/x86/include/asm/cmpxchg.h @@ -212,8 +212,9 @@ extern void __add_wrong_size(void) default: \ __cmpxchg_wrong_size(); \ } \ - *_old = __old; \ - success; \ + if (unlikely(!success)) \ + *_old = __old; \ + likely(success); \ }) #define __try_cmpxchg(ptr, pold, new, size) \
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-03-25 19:40 +0100 |
| Message-ID | <toYLv-4H9-3@gated-at.bofh.it> |
| In reply to | #1609208 |
On Sat, Mar 25, 2017 at 11:20 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> Added above, a few bytes smaller than the shiny new one actually.
Hmm. Sad. The label approach looked like it would match the semantics
of cmpxchg perfectly, but it's not as optimal as it superficially
would have seemed.
And I assume that register allocation etc is different enough that
there's no sane way to diff the asm to see what changed.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-03-24 21:30 +0100 |
| Message-ID | <toE0p-6Pe-1@gated-at.bofh.it> |
| In reply to | #1608865 |
On Fri, Mar 24, 2017 at 11:45:46AM -0700, Andy Lutomirski wrote: > After playing with it a bit, I found some of the problem: you're > passing val into EXCEPTION_VALUE, which keeps it live. If I get rid > of that, the generated code is great. Right, so I needed that because I land on ud2 through 2 different paths: - newly saturated - use-after-free And the exception handler can figure out which of the two by looking at the variable, but then of course, it needs to be life. For the full horror of how to do this, look here: http://paste.debian.net/924190/ But I didn't just show you that, so you can't blame me for any damage that might've done you.
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2017-03-24 21:30 +0100 |
| Message-ID | <toE0q-6Pe-17@gated-at.bofh.it> |
| In reply to | #1608953 |
On Fri, Mar 24, 2017 at 1:22 PM, Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, Mar 24, 2017 at 11:45:46AM -0700, Andy Lutomirski wrote: >> After playing with it a bit, I found some of the problem: you're >> passing val into EXCEPTION_VALUE, which keeps it live. If I get rid >> of that, the generated code is great. > > Right, so I needed that because I land on ud2 through 2 different paths: > > - newly saturated > - use-after-free > > And the exception handler can figure out which of the two by looking at > the variable, but then of course, it needs to be life. > > For the full horror of how to do this, look here: > > http://paste.debian.net/924190/ > > But I didn't just show you that, so you can't blame me for any damage > that might've done you. Wow, that's horrible. Could this not be done by looking at flags instead of regs? For that matter, you're effectively comparing to -1 and 0. I'm not really sure it would be faster, but you could plausibly add one then subtract one again and get the full picture just from flags and a single comparison?
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-03-24 22:10 +0100 |
| Message-ID | <toED7-7kH-13@gated-at.bofh.it> |
| In reply to | #1608956 |
On Fri, Mar 24, 2017 at 01:27:49PM -0700, Andy Lutomirski wrote: > On Fri, Mar 24, 2017 at 1:22 PM, Peter Zijlstra <peterz@infradead.org> wrote: > > On Fri, Mar 24, 2017 at 11:45:46AM -0700, Andy Lutomirski wrote: > >> After playing with it a bit, I found some of the problem: you're > >> passing val into EXCEPTION_VALUE, which keeps it live. If I get rid > >> of that, the generated code is great. > > > > Right, so I needed that because I land on ud2 through 2 different paths: > > > > - newly saturated > > - use-after-free > > > > And the exception handler can figure out which of the two by looking at > > the variable, but then of course, it needs to be life. > > > > For the full horror of how to do this, look here: > > > > http://paste.debian.net/924190/ > > > > But I didn't just show you that, so you can't blame me for any damage > > that might've done you. > > Wow, that's horrible. Could this not be done by looking at flags > instead of regs? Well, the EXCEPTION_HANDLER() thing is something ARM/ARM64 could also implement.
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-03-24 20:10 +0100 |
| Message-ID | <toCL0-5Yy-25@gated-at.bofh.it> |
| In reply to | #1608703 |
On Fri, Mar 24, 2017 at 10:23 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> I tried a few variants, but nothing really made it better.
So I really hate how your thing has two return values, and fakes the
second one using the pointer value. I dislike it for two different
reasons:
- it's bad for type checking: it makes the "real" pointer value and
the "old value" pointer value both be pointers of the same type
- I think it's part of the reason why gcc easily generates crap code.
And I think the problem is fundamental to your interface.
So how about we change the interface entirely, with the goal being
both type safety and good code generation?
In particular, let's make the interface something that might be
optimal given possible future gcc improvements, like the ability to
use "asm goto" with outputs. We can't do that today, but maybe some
day...
So I would suggest that the format of the "try_cmpxhg()" thing be
something like this:
#define try_cmpxchg(ptr, value, new, success_label) ({ \
bool __txchg_success; \
__typeof__(*(ptr)) __old; \
asm volatile("lock cmpxchgl %3, %1" \
: "=@ccz" (__txchg_success), \
"+m" (*ptr), \
"=a" (__old) \
: "r" (new), \
"2" (value) \
: "memory"); \
if (__txchg_success) goto success_label; \
__old; })
which seems to be fairly natural.
Then you do your refcount loop (or pretty much *any* cmpxchg loop) with
for (;;) {
if (unlikely(val == UINT_MAX))
goto saturated;
if (unlikely(!val))
goto use_after_free;
new = val + 1;
val = try_cmpxchg(r, val, new, success);
}
success:
return;
saturated: use_after_free: .. whatever error handling ..
and I think gcc should generate reasonable code.
Hmm?
NOTE! I would suggest that this odd "macro with a success label" model
of try_cmpxchg() never be used for something that people are expected
to use directly. So I don't think "normal" code should use this label
form.
But it's useful as a helper function that is then used to implement
the "real" ABI, ie to implement that "refcount_inc()" and other things
like that..
Linus
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-03-24 21:50 +0100 |
| Message-ID | <toEjL-6Xy-15@gated-at.bofh.it> |
| In reply to | #1608893 |
On Fri, Mar 24, 2017 at 12:08:32PM -0700, Linus Torvalds wrote: > On Fri, Mar 24, 2017 at 10:23 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > > > I tried a few variants, but nothing really made it better. > > So I really hate how your thing has two return values, and fakes the > second one using the pointer value. Inspired by C11 I'm afraid.. > So how about we change the interface entirely, with the goal being > both type safety and good code generation? I certainly like it better, but so far I'm having trouble reproducing your results. What compiler version are you on?
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-03-24 22:00 +0100 |
| Message-ID | <toEts-71u-11@gated-at.bofh.it> |
| In reply to | #1608967 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, Mar 24, 2017 at 1:46 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> I certainly like it better, but so far I'm having trouble reproducing
> your results. What compiler version are you on?
I have:
gcc version 6.3.1 20161221 (Red Hat 6.3.1-1) (GCC)
from
gcc-6.3.1-1.fc24.x86_64
and I'm attaching the edited form of your test-case, just so that
we're on the exact same page.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-03-27 14:20 +0200 |
| Message-ID | <tpBMS-7V7-17@gated-at.bofh.it> |
| In reply to | #1608382 |
On Fri, Mar 24, 2017 at 01:44:00PM +0100, Dmitry Vyukov wrote:
> Hi,
>
> I've come across:
>
> commit a9ebf306f52c756c4f9e50ee9a60cd6389d71344
> Author: Peter Zijlstra
> Date: Wed Feb 1 16:39:38 2017 +0100
> locking/atomic: Introduce atomic_try_cmpxchg()
>
> The primitive has subtle difference with all other implementation that
> I know of, and can lead to very subtle bugs. Some time ago I've spent
> several days debugging a memory corruption caused by similar
> implementation.
OK, so how about this?
---
Subject: atomic: Fix try_cmpxchg semantics
From: Peter Zijlstra <peterz@infradead.org>
Date: Mon Mar 27 13:54:38 CEST 2017
Dmitry noted that the new try_cmpxchg() primitive is broken when the
old pointer doesn't point to local stack.
He writes: "Consider a classical lock-free stack push:
node->next = atomic_read(&head);
do {
} while (!atomic_try_cmpxchg(&head, &node->next, node));
This code is broken with the current implementation, the problem is
with unconditional update of *__po.
In case of success it writes the same value back into *__po, but in
case of cmpxchg success we might have lose ownership of some memory
locations and potentially over what __po has pointed to. The same
holds for the re-read of *__po. "
He also points out that this makes it surprisingly different from the
similar C/C++ atomic operation.
After investigating the code-gen differences caused by this patch; and
a number of alternatives (Linus dislikes this interface lots), we
arrived at these results (size x86_64-defconfig/vmlinux):
GCC-6.3.0:
10735757 cmpxchg
10726413 try_cmpxchg
10730509 try_cmpxchg + patch
10730445 try_cmpxchg-linus
GCC-7 (20170327):
10709514 cmpxchg
10704266 try_cmpxchg
10704266 try_cmpxchg + patch
10704394 try_cmpxchg-linus
From this we see that the patch has the advantage of better code-gen on
GCC-7 and keeps the interface roughly consistent with the C language
variant.
Fixes: a9ebf306f52c ("locking/atomic: Introduce atomic_try_cmpxchg()")
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/include/asm/cmpxchg.h | 5 +++--
include/linux/atomic.h | 16 ++++++++++------
2 files changed, 13 insertions(+), 8 deletions(-)
--- a/arch/x86/include/asm/cmpxchg.h
+++ b/arch/x86/include/asm/cmpxchg.h
@@ -212,8 +212,9 @@ extern void __add_wrong_size(void)
default: \
__cmpxchg_wrong_size(); \
} \
- *_old = __old; \
- success; \
+ if (unlikely(!success)) \
+ *_old = __old; \
+ likely(success); \
})
#define __try_cmpxchg(ptr, pold, new, size) \
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -428,9 +428,11 @@
#define __atomic_try_cmpxchg(type, _p, _po, _n) \
({ \
typeof(_po) __po = (_po); \
- typeof(*(_po)) __o = *__po; \
- *__po = atomic_cmpxchg##type((_p), __o, (_n)); \
- (*__po == __o); \
+ typeof(*(_po)) __r, __o = *__po; \
+ __r = atomic_cmpxchg##type((_p), __o, (_n)); \
+ if (unlikely(__r != __o)) \
+ *__po = __r; \
+ likely(__r == __o); \
})
#define atomic_try_cmpxchg(_p, _po, _n) __atomic_try_cmpxchg(, _p, _po, _n)
@@ -1022,9 +1024,11 @@ static inline int atomic_dec_if_positive
#define __atomic64_try_cmpxchg(type, _p, _po, _n) \
({ \
typeof(_po) __po = (_po); \
- typeof(*(_po)) __o = *__po; \
- *__po = atomic64_cmpxchg##type((_p), __o, (_n)); \
- (*__po == __o); \
+ typeof(*(_po)) __r, __o = *__po; \
+ __r = atomic64_cmpxchg##type((_p), __o, (_n)); \
+ if (unlikely(__r != __o)) \
+ *__po = __r; \
+ likely(__r == __o); \
})
#define atomic64_try_cmpxchg(_p, _po, _n) __atomic64_try_cmpxchg(, _p, _po, _n)
[toc] | [prev] | [next] | [standalone]
| From | Dmitry Vyukov <dvyukov@google.com> |
|---|---|
| Date | 2017-03-27 15:50 +0200 |
| Message-ID | <tpDbX-nU-9@gated-at.bofh.it> |
| In reply to | #1609767 |
On Mon, Mar 27, 2017 at 2:16 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 24, 2017 at 01:44:00PM +0100, Dmitry Vyukov wrote:
>> Hi,
>>
>> I've come across:
>>
>> commit a9ebf306f52c756c4f9e50ee9a60cd6389d71344
>> Author: Peter Zijlstra
>> Date: Wed Feb 1 16:39:38 2017 +0100
>> locking/atomic: Introduce atomic_try_cmpxchg()
>>
>> The primitive has subtle difference with all other implementation that
>> I know of, and can lead to very subtle bugs. Some time ago I've spent
>> several days debugging a memory corruption caused by similar
>> implementation.
>
> OK, so how about this?
>
> ---
> Subject: atomic: Fix try_cmpxchg semantics
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Mon Mar 27 13:54:38 CEST 2017
>
> Dmitry noted that the new try_cmpxchg() primitive is broken when the
> old pointer doesn't point to local stack.
>
> He writes: "Consider a classical lock-free stack push:
>
> node->next = atomic_read(&head);
> do {
> } while (!atomic_try_cmpxchg(&head, &node->next, node));
>
> This code is broken with the current implementation, the problem is
> with unconditional update of *__po.
>
> In case of success it writes the same value back into *__po, but in
> case of cmpxchg success we might have lose ownership of some memory
> locations and potentially over what __po has pointed to. The same
> holds for the re-read of *__po. "
>
> He also points out that this makes it surprisingly different from the
> similar C/C++ atomic operation.
>
> After investigating the code-gen differences caused by this patch; and
> a number of alternatives (Linus dislikes this interface lots), we
> arrived at these results (size x86_64-defconfig/vmlinux):
>
> GCC-6.3.0:
>
> 10735757 cmpxchg
> 10726413 try_cmpxchg
> 10730509 try_cmpxchg + patch
> 10730445 try_cmpxchg-linus
>
> GCC-7 (20170327):
>
> 10709514 cmpxchg
> 10704266 try_cmpxchg
> 10704266 try_cmpxchg + patch
> 10704394 try_cmpxchg-linus
>
> From this we see that the patch has the advantage of better code-gen on
> GCC-7 and keeps the interface roughly consistent with the C language
> variant.
>
> Fixes: a9ebf306f52c ("locking/atomic: Introduce atomic_try_cmpxchg()")
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> arch/x86/include/asm/cmpxchg.h | 5 +++--
> include/linux/atomic.h | 16 ++++++++++------
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> --- a/arch/x86/include/asm/cmpxchg.h
> +++ b/arch/x86/include/asm/cmpxchg.h
> @@ -212,8 +212,9 @@ extern void __add_wrong_size(void)
> default: \
> __cmpxchg_wrong_size(); \
> } \
> - *_old = __old; \
> - success; \
> + if (unlikely(!success)) \
> + *_old = __old; \
> + likely(success); \
> })
>
> #define __try_cmpxchg(ptr, pold, new, size) \
> --- a/include/linux/atomic.h
> +++ b/include/linux/atomic.h
> @@ -428,9 +428,11 @@
> #define __atomic_try_cmpxchg(type, _p, _po, _n) \
> ({ \
> typeof(_po) __po = (_po); \
> - typeof(*(_po)) __o = *__po; \
> - *__po = atomic_cmpxchg##type((_p), __o, (_n)); \
> - (*__po == __o); \
> + typeof(*(_po)) __r, __o = *__po; \
> + __r = atomic_cmpxchg##type((_p), __o, (_n)); \
> + if (unlikely(__r != __o)) \
> + *__po = __r; \
> + likely(__r == __o); \
> })
>
> #define atomic_try_cmpxchg(_p, _po, _n) __atomic_try_cmpxchg(, _p, _po, _n)
> @@ -1022,9 +1024,11 @@ static inline int atomic_dec_if_positive
> #define __atomic64_try_cmpxchg(type, _p, _po, _n) \
> ({ \
> typeof(_po) __po = (_po); \
> - typeof(*(_po)) __o = *__po; \
> - *__po = atomic64_cmpxchg##type((_p), __o, (_n)); \
> - (*__po == __o); \
> + typeof(*(_po)) __r, __o = *__po; \
> + __r = atomic64_cmpxchg##type((_p), __o, (_n)); \
> + if (unlikely(__r != __o)) \
> + *__po = __r; \
> + likely(__r == __o); \
> })
>
> #define atomic64_try_cmpxchg(_p, _po, _n) __atomic64_try_cmpxchg(, _p, _po, _n)
Acked-by: Dmitry Vyukov <dvyukov@google.com>
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web