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

Groups > linux.kernel > #1223032 > unrolled thread

[PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops in native code

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops in native code Waiman Long <Waiman.Long@hpe.com> - 2015-09-11 20:40 +0200
    Re: [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops in  native code Davidlohr Bueso <dave@stgolabs.net> - 2015-09-12 00:30 +0200
      Re: [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops in  native code Peter Zijlstra <peterz@infradead.org> - 2015-09-14 14:10 +0200
        Re: [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops  in native code Waiman Long <waiman.long@hpe.com> - 2015-09-14 20:50 +0200
      Re: [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops  in native code Waiman Long <waiman.long@hpe.com> - 2015-09-14 17:20 +0200

#1223032 — [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops in native code

This patch replaces the cmpxchg() and xchg() calls in the native
qspinlock code with more relaxed versions of those calls to enable
other architectures to adopt queued spinlocks with less performance
overhead.

Signed-off-by: Waiman Long <Waiman.Long@hpe.com>
---
 arch/x86/include/asm/qspinlock.h |    2 +-
 include/asm-generic/qspinlock.h  |    6 +++---
 kernel/locking/qspinlock.c       |   21 +++++++++++++++++----
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
index 9d51fae..053e70d 100644
--- a/arch/x86/include/asm/qspinlock.h
+++ b/arch/x86/include/asm/qspinlock.h
@@ -46,7 +46,7 @@ static inline bool virt_queued_spin_lock(struct qspinlock *lock)
 	if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
 		return false;
 
-	while (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) != 0)
+	while (atomic_cmpxchg_acquire(&lock->val, 0, _Q_LOCKED_VAL) != 0)
 		cpu_relax();
 
 	return true;
diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
index 83bfb87..efbd1fd 100644
--- a/include/asm-generic/qspinlock.h
+++ b/include/asm-generic/qspinlock.h
@@ -62,7 +62,7 @@ static __always_inline int queued_spin_is_contended(struct qspinlock *lock)
 static __always_inline int queued_spin_trylock(struct qspinlock *lock)
 {
 	if (!atomic_read(&lock->val) &&
-	   (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) == 0))
+	   (atomic_cmpxchg_acquire(&lock->val, 0, _Q_LOCKED_VAL) == 0))
 		return 1;
 	return 0;
 }
@@ -77,7 +77,7 @@ static __always_inline void queued_spin_lock(struct qspinlock *lock)
 {
 	u32 val;
 
-	val = atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL);
+	val = atomic_cmpxchg_acquire(&lock->val, 0, _Q_LOCKED_VAL);
 	if (likely(val == 0))
 		return;
 	queued_spin_lock_slowpath(lock, val);
@@ -93,7 +93,7 @@ static __always_inline void queued_spin_unlock(struct qspinlock *lock)
 	/*
 	 * smp_mb__before_atomic() in order to guarantee release semantics
 	 */
-	smp_mb__before_atomic_dec();
+	smp_mb__before_atomic();
 	atomic_sub(_Q_LOCKED_VAL, &lock->val);
 }
 #endif
diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index 337c881..28a15c7 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -176,7 +176,12 @@ static __always_inline u32 xchg_tail(struct qspinlock *lock, u32 tail)
 {
 	struct __qspinlock *l = (void *)lock;
 
-	return (u32)xchg(&l->tail, tail >> _Q_TAIL_OFFSET) << _Q_TAIL_OFFSET;
+	/*
+	 * Use release semantics to make sure that the MCS node is properly
+	 * initialized before changing the tail code.
+	 */
+	return (u32)xchg_release(&l->tail,
+				 tail >> _Q_TAIL_OFFSET) << _Q_TAIL_OFFSET;
 }
 
 #else /* _Q_PENDING_BITS == 8 */
@@ -208,7 +213,11 @@ static __always_inline u32 xchg_tail(struct qspinlock *lock, u32 tail)
 
 	for (;;) {
 		new = (val & _Q_LOCKED_PENDING_MASK) | tail;
-		old = atomic_cmpxchg(&lock->val, val, new);
+		/*
+		 * Use release semantics to make sure that the MCS node is
+		 * properly initialized before changing the tail code.
+		 */
+		old = atomic_cmpxchg_release(&lock->val, val, new);
 		if (old == val)
 			break;
 
@@ -319,7 +328,7 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
 		if (val == new)
 			new |= _Q_PENDING_VAL;
 
-		old = atomic_cmpxchg(&lock->val, val, new);
+		old = atomic_cmpxchg_acquire(&lock->val, val, new);
 		if (old == val)
 			break;
 
@@ -426,7 +435,11 @@ queue:
 			set_locked(lock);
 			break;
 		}
-		old = atomic_cmpxchg(&lock->val, val, _Q_LOCKED_VAL);
+		/*
+		 * The smp_load_acquire() call above has provided the necessary
+		 * acquire semantics required for locking.
+		 */
+		old = atomic_cmpxchg_relaxed(&lock->val, val, _Q_LOCKED_VAL);
 		if (old == val)
 			goto release;	/* No contention */
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1223135 — Re: [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops in native code

On Fri, 11 Sep 2015, Waiman Long wrote:

>@@ -46,7 +46,7 @@ static inline bool virt_queued_spin_lock(struct qspinlock *lock)
> 	if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
> 		return false;
>
>-	while (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) != 0)
>+	while (atomic_cmpxchg_acquire(&lock->val, 0, _Q_LOCKED_VAL) != 0)
> 		cpu_relax();

This code has changed with Peter's recent ccas fix. And the whole virt_queued_spin_lock()
thing will now be under pv configs. So this doesn't apply to native code anymore, so it
looks like it should be dropped altogether.

Thanks,
Davidlohr
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1224054 — Re: [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops in native code

On Fri, Sep 11, 2015 at 03:27:44PM -0700, Davidlohr Bueso wrote:
> On Fri, 11 Sep 2015, Waiman Long wrote:
> 
> >@@ -46,7 +46,7 @@ static inline bool virt_queued_spin_lock(struct qspinlock *lock)
> >	if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
> >		return false;
> >
> >-	while (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) != 0)
> >+	while (atomic_cmpxchg_acquire(&lock->val, 0, _Q_LOCKED_VAL) != 0)
> >		cpu_relax();
> 
> This code has changed with Peter's recent ccas fix. And the whole virt_queued_spin_lock()
> thing will now be under pv configs. So this doesn't apply to native code anymore, so it
> looks like it should be dropped altogether.

Yeah, it also doesn't make sense, this ix x86 arch code, x86 cannot do
cmpxchg_acquire. Then again, I suppose we could argue its of
documentation value..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1224403 — Re: [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops in native code

On 09/14/2015 08:06 AM, Peter Zijlstra wrote:
> On Fri, Sep 11, 2015 at 03:27:44PM -0700, Davidlohr Bueso wrote:
>> On Fri, 11 Sep 2015, Waiman Long wrote:
>>
>>> @@ -46,7 +46,7 @@ static inline bool virt_queued_spin_lock(struct qspinlock *lock)
>>> 	if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
>>> 		return false;
>>>
>>> -	while (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) != 0)
>>> +	while (atomic_cmpxchg_acquire(&lock->val, 0, _Q_LOCKED_VAL) != 0)
>>> 		cpu_relax();
>> This code has changed with Peter's recent ccas fix. And the whole virt_queued_spin_lock()
>> thing will now be under pv configs. So this doesn't apply to native code anymore, so it
>> looks like it should be dropped altogether.
> Yeah, it also doesn't make sense, this ix x86 arch code, x86 cannot do
> cmpxchg_acquire. Then again, I suppose we could argue its of
> documentation value..

Yes, it is to be consistent with the change in asm_generic qspinlock.h. 
We can certainly skip that.

Cheers,
Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1224243 — Re: [PATCH v6 1/6] locking/qspinlock: relaxes cmpxchg & xchg ops in native code

On 09/11/2015 06:27 PM, Davidlohr Bueso wrote:
> On Fri, 11 Sep 2015, Waiman Long wrote:
>
>> @@ -46,7 +46,7 @@ static inline bool virt_queued_spin_lock(struct 
>> qspinlock *lock)
>>     if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
>>         return false;
>>
>> -    while (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) != 0)
>> +    while (atomic_cmpxchg_acquire(&lock->val, 0, _Q_LOCKED_VAL) != 0)
>>         cpu_relax();
>
> This code has changed with Peter's recent ccas fix. And the whole 
> virt_queued_spin_lock()
> thing will now be under pv configs. So this doesn't apply to native 
> code anymore, so it
> looks like it should be dropped altogether.
>
> Thanks,
> Davidlohr

You are right. Patch 1 needs to be updated on top of PeterZ latest patch.

Cheers,
Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web