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

Groups > linux.kernel > #1259848 > unrolled thread

[PATCH tip/locking/core v9 0/6] locking/qspinlock: Enhance pvqspinlock

Back to article view | Back to linux.kernel

Contents

  [PATCH tip/locking/core v9 0/6] locking/qspinlock: Enhance pvqspinlock  Waiman Long <Waiman.Long@hpe.com> - 2015-10-31 00:30 +0100
    [PATCH tip/locking/core v9 1/6] locking/qspinlock: Use _acquire/_release versions of cmpxchg & xchg Waiman Long <Waiman.Long@hpe.com> - 2015-10-31 00:30 +0100
    [PATCH tip/locking/core v9 2/6] locking/qspinlock: prefetch next node cacheline Waiman Long <Waiman.Long@hpe.com> - 2015-10-31 00:30 +0100
      Re: [PATCH tip/locking/core v9 2/6] locking/qspinlock: prefetch next  node cacheline Peter Zijlstra <peterz@infradead.org> - 2015-11-02 17:40 +0100
        Re: [PATCH tip/locking/core v9 2/6] locking/qspinlock: prefetch next  node cacheline Peter Zijlstra <peterz@infradead.org> - 2015-11-03 00:00 +0100
          Re: [PATCH tip/locking/core v9 2/6] locking/qspinlock: prefetch next  node cacheline Waiman Long <waiman.long@hpe.com> - 2015-11-05 17:50 +0100
            Re: [PATCH tip/locking/core v9 2/6] locking/qspinlock: prefetch next  node cacheline Peter Zijlstra <peterz@infradead.org> - 2015-11-05 18:00 +0100
        Re: [PATCH tip/locking/core v9 2/6] locking/qspinlock: prefetch next  node cacheline Waiman Long <waiman.long@hpe.com> - 2015-11-05 17:10 +0100
          Re: [PATCH tip/locking/core v9 2/6] locking/qspinlock: prefetch next  node cacheline Peter Zijlstra <peterz@infradead.org> - 2015-11-05 17:40 +0100
            Re: [PATCH tip/locking/core v9 2/6] locking/qspinlock: prefetch next  node cacheline Waiman Long <waiman.long@hpe.com> - 2015-11-05 18:00 +0100
    [PATCH tip/locking/core v9 5/6] locking/pvqspinlock: Allow 1 lock stealing attempt Waiman Long <Waiman.Long@hpe.com> - 2015-10-31 00:30 +0100
      Re: [PATCH tip/locking/core v9 5/6] locking/pvqspinlock: Allow 1  lock stealing attempt Peter Zijlstra <peterz@infradead.org> - 2015-11-06 16:00 +0100
        Re: [PATCH tip/locking/core v9 5/6] locking/pvqspinlock: Allow 1  lock stealing attempt Waiman Long <waiman.long@hpe.com> - 2015-11-06 18:50 +0100
          Re: [PATCH tip/locking/core v9 5/6] locking/pvqspinlock: Allow 1  lock stealing attempt Peter Zijlstra <peterz@infradead.org> - 2015-11-09 18:40 +0100
            Re: [PATCH tip/locking/core v9 5/6] locking/pvqspinlock: Allow 1  lock stealing attempt Waiman Long <waiman.long@hpe.com> - 2015-11-09 21:00 +0100
    [PATCH tip/locking/core v9 6/6] locking/pvqspinlock: Queue node adaptive spinning Waiman Long <Waiman.Long@hpe.com> - 2015-10-31 00:30 +0100
      Re: [PATCH tip/locking/core v9 6/6] locking/pvqspinlock: Queue node  adaptive spinning Peter Zijlstra <peterz@infradead.org> - 2015-11-06 16:10 +0100
        Re: [PATCH tip/locking/core v9 6/6] locking/pvqspinlock: Queue node  adaptive spinning Waiman Long <waiman.long@hpe.com> - 2015-11-06 19:00 +0100
          Re: [PATCH tip/locking/core v9 6/6] locking/pvqspinlock: Queue node  adaptive spinning Peter Zijlstra <peterz@infradead.org> - 2015-11-06 21:40 +0100
            Re: [PATCH tip/locking/core v9 6/6] locking/pvqspinlock: Queue node  adaptive spinning Waiman Long <waiman.long@hpe.com> - 2015-11-09 18:00 +0100
              Re: [PATCH tip/locking/core v9 6/6] locking/pvqspinlock: Queue node  adaptive spinning Peter Zijlstra <peterz@infradead.org> - 2015-11-09 18:40 +0100
    [PATCH tip/locking/core v9 3/6] locking/pvqspinlock, x86: Optimize PV unlock code path Waiman Long <Waiman.Long@hpe.com> - 2015-10-31 00:30 +0100
    [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect slowpath lock statistics Waiman Long <Waiman.Long@hpe.com> - 2015-10-31 00:30 +0100
      Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect  slowpath lock statistics Peter Zijlstra <peterz@infradead.org> - 2015-11-02 17:50 +0100
        Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect  slowpath lock statistics Waiman Long <waiman.long@hpe.com> - 2015-11-05 17:30 +0100
          Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect  slowpath lock statistics Peter Zijlstra <peterz@infradead.org> - 2015-11-05 17:50 +0100
            Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect  slowpath lock statistics Waiman Long <waiman.long@hpe.com> - 2015-11-05 18:00 +0100
              Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect  slowpath lock statistics Peter Zijlstra <peterz@infradead.org> - 2015-11-05 18:10 +0100
                Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect  slowpath lock statistics Waiman Long <waiman.long@hpe.com> - 2015-11-05 18:40 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1265889 — Re: [PATCH tip/locking/core v9 6/6] locking/pvqspinlock: Queue node adaptive spinning

On Mon, Nov 09, 2015 at 11:51:20AM -0500, Waiman Long wrote:
> On 11/06/2015 03:37 PM, Peter Zijlstra wrote:
> >On Fri, Nov 06, 2015 at 12:54:06PM -0500, Waiman Long wrote:
> >>>>+static void pv_wait_node(struct mcs_spinlock *node, struct mcs_spinlock *prev)
> >>>>  {
> >>>>  	struct pv_node *pn = (struct pv_node *)node;
> >>>>+	struct pv_node *pp = (struct pv_node *)prev;
> >>>>  	int waitcnt = 0;
> >>>>  	int loop;
> >>>>+	bool wait_early;
> >>>>
> >>>>  	/* waitcnt processing will be compiled out if !QUEUED_LOCK_STAT */
> >>>>  	for (;; waitcnt++) {
> >>>>-		for (loop = SPIN_THRESHOLD; loop; loop--) {
> >>>>+		for (wait_early = false, loop = SPIN_THRESHOLD; loop; loop--) {
> >>>>  			if (READ_ONCE(node->locked))
> >>>>  				return;
> >>>>+			if (pv_wait_early(pp, loop)) {
> >>>>+				wait_early = true;
> >>>>+				break;
> >>>>+			}
> >>>>  			cpu_relax();
> >>>>  		}
> >>>>
> >>>So if prev points to another node, it will never see vcpu_running. Was
> >>>that fully intended?
> >>I had added code in pv_wait_head_or_lock to set the state appropriately for
> >>the queue head vCPU.
> >Yes, but that's the head, for nodes we'll always have halted or hashed.
> 
> The node state was initialized to be vcpu_running. In pv_wait_node(), it
> will be changed to vcpu_halted before sleeping and back to vcpu_running
> after that. So it is not true that it is either halted or hashed.

Durh,.. I mixed up pv_wait_node() and pv_wait_head() I think. Sorry for
the noise.
--
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]

#1259855 — [PATCH tip/locking/core v9 3/6] locking/pvqspinlock, x86: Optimize PV unlock code path

The unlock function in queued spinlocks was optimized for better
performance on bare metal systems at the expense of virtualized guests.

For x86-64 systems, the unlock call needs to go through a
PV_CALLEE_SAVE_REGS_THUNK() which saves and restores 8 64-bit
registers before calling the real __pv_queued_spin_unlock()
function. The thunk code may also be in a separate cacheline from
__pv_queued_spin_unlock().

This patch optimizes the PV unlock code path by:
 1) Moving the unlock slowpath code from the fastpath into a separate
    __pv_queued_spin_unlock_slowpath() function to make the fastpath
    as simple as possible..
 2) For x86-64, hand-coded an assembly function to combine the register
    saving thunk code with the fastpath code. Only registers that
    are used in the fastpath will be saved and restored. If the
    fastpath fails, the slowpath function will be called via another
    PV_CALLEE_SAVE_REGS_THUNK(). For 32-bit, it falls back to the C
    __pv_queued_spin_unlock() code as the thunk saves and restores
    only one 32-bit register.

With a microbenchmark of 5M lock-unlock loop, the table below shows
the execution times before and after the patch with different number
of threads in a VM running on a 32-core Westmere-EX box with x86-64
4.2-rc1 based kernels:

  Threads	Before patch	After patch	% Change
  -------	------------	-----------	--------
     1		   134.1 ms	  119.3 ms	  -11%
     2		   1286  ms	   953  ms	  -26%
     3		   3715  ms	  3480  ms	  -6.3%
     4		   4092  ms	  3764  ms	  -8.0%

Signed-off-by: Waiman Long <Waiman.Long@hpe.com>
---
 arch/x86/include/asm/qspinlock_paravirt.h |   59 +++++++++++++++++++++++++++++
 kernel/locking/qspinlock_paravirt.h       |   43 +++++++++++++--------
 2 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/arch/x86/include/asm/qspinlock_paravirt.h b/arch/x86/include/asm/qspinlock_paravirt.h
index b002e71..9f92c18 100644
--- a/arch/x86/include/asm/qspinlock_paravirt.h
+++ b/arch/x86/include/asm/qspinlock_paravirt.h
@@ -1,6 +1,65 @@
 #ifndef __ASM_QSPINLOCK_PARAVIRT_H
 #define __ASM_QSPINLOCK_PARAVIRT_H
 
+/*
+ * For x86-64, PV_CALLEE_SAVE_REGS_THUNK() saves and restores 8 64-bit
+ * registers. For i386, however, only 1 32-bit register needs to be saved
+ * and restored. So an optimized version of __pv_queued_spin_unlock() is
+ * hand-coded for 64-bit, but it isn't worthwhile to do it for 32-bit.
+ */
+#ifdef CONFIG_64BIT
+
+PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath);
+#define __pv_queued_spin_unlock	__pv_queued_spin_unlock
+#define PV_UNLOCK		"__raw_callee_save___pv_queued_spin_unlock"
+#define PV_UNLOCK_SLOWPATH	"__raw_callee_save___pv_queued_spin_unlock_slowpath"
+
+/*
+ * Optimized assembly version of __raw_callee_save___pv_queued_spin_unlock
+ * which combines the registers saving trunk and the body of the following
+ * C code:
+ *
+ * void __pv_queued_spin_unlock(struct qspinlock *lock)
+ * {
+ *	struct __qspinlock *l = (void *)lock;
+ *	u8 lockval = cmpxchg(&l->locked, _Q_LOCKED_VAL, 0);
+ *
+ *	if (likely(lockval == _Q_LOCKED_VAL))
+ *		return;
+ *	pv_queued_spin_unlock_slowpath(lock, lockval);
+ * }
+ *
+ * For x86-64,
+ *   rdi = lock              (first argument)
+ *   rsi = lockval           (second argument)
+ *   rdx = internal variable (set to 0)
+ */
+asm    (".pushsection .text;"
+	".globl " PV_UNLOCK ";"
+	".align 4,0x90;"
+	PV_UNLOCK ": "
+	"push  %rdx;"
+	"mov   $0x1,%eax;"
+	"xor   %edx,%edx;"
+	"lock cmpxchg %dl,(%rdi);"
+	"cmp   $0x1,%al;"
+	"jne   .slowpath;"
+	"pop   %rdx;"
+	"ret;"
+	".slowpath: "
+	"push   %rsi;"
+	"movzbl %al,%esi;"
+	"call " PV_UNLOCK_SLOWPATH ";"
+	"pop    %rsi;"
+	"pop    %rdx;"
+	"ret;"
+	".size " PV_UNLOCK ", .-" PV_UNLOCK ";"
+	".popsection");
+
+#else /* CONFIG_64BIT */
+
+extern void __pv_queued_spin_unlock(struct qspinlock *lock);
 PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock);
 
+#endif /* CONFIG_64BIT */
 #endif
diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
index f0450ff..4bd323d 100644
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -308,23 +308,14 @@ static void pv_wait_head(struct qspinlock *lock, struct mcs_spinlock *node)
 }
 
 /*
- * PV version of the unlock function to be used in stead of
- * queued_spin_unlock().
+ * PV versions of the unlock fastpath and slowpath functions to be used
+ * instead of queued_spin_unlock().
  */
-__visible void __pv_queued_spin_unlock(struct qspinlock *lock)
+__visible void
+__pv_queued_spin_unlock_slowpath(struct qspinlock *lock, u8 locked)
 {
 	struct __qspinlock *l = (void *)lock;
 	struct pv_node *node;
-	u8 locked;
-
-	/*
-	 * We must not unlock if SLOW, because in that case we must first
-	 * unhash. Otherwise it would be possible to have multiple @lock
-	 * entries, which would be BAD.
-	 */
-	locked = cmpxchg(&l->locked, _Q_LOCKED_VAL, 0);
-	if (likely(locked == _Q_LOCKED_VAL))
-		return;
 
 	if (unlikely(locked != _Q_SLOW_VAL)) {
 		WARN(!debug_locks_silent,
@@ -363,12 +354,32 @@ __visible void __pv_queued_spin_unlock(struct qspinlock *lock)
 	 */
 	pv_kick(node->cpu);
 }
+
 /*
  * Include the architecture specific callee-save thunk of the
  * __pv_queued_spin_unlock(). This thunk is put together with
- * __pv_queued_spin_unlock() near the top of the file to make sure
- * that the callee-save thunk and the real unlock function are close
- * to each other sharing consecutive instruction cachelines.
+ * __pv_queued_spin_unlock() to make the callee-save thunk and the real unlock
+ * function close to each other sharing consecutive instruction cachelines.
+ * Alternatively, architecture specific version of __pv_queued_spin_unlock()
+ * can be defined.
  */
 #include <asm/qspinlock_paravirt.h>
 
+#ifndef __pv_queued_spin_unlock
+__visible void __pv_queued_spin_unlock(struct qspinlock *lock)
+{
+	struct __qspinlock *l = (void *)lock;
+	u8 locked;
+
+	/*
+	 * We must not unlock if SLOW, because in that case we must first
+	 * unhash. Otherwise it would be possible to have multiple @lock
+	 * entries, which would be BAD.
+	 */
+	locked = cmpxchg(&l->locked, _Q_LOCKED_VAL, 0);
+	if (likely(locked == _Q_LOCKED_VAL))
+		return;
+
+	__pv_queued_spin_unlock_slowpath(lock, locked);
+}
+#endif /* __pv_queued_spin_unlock */
-- 
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] | [prev] | [next] | [standalone]

#1259856 — [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect slowpath lock statistics

This patch enables the accumulation of kicking and waiting related
PV qspinlock statistics when the new QUEUED_LOCK_STAT configuration
option is selected. It also enables the collection of data which
enable us to calculate the kicking and wakeup latencies which have
a heavy dependency on the CPUs being used.

The statistical counters are per-cpu variables to minimize the
performance overhead in their updates. These counters are exported
via the sysfs filesystem under the /sys/kernel/qlockstat directory.
When the corresponding sysfs files are read, summation and computing
of the required data are then performed.

The measured latencies for different CPUs are:

	CPU		Wakeup		Kicking
	---		------		-------
	Haswell-EX	63.6us		 7.4us
	Westmere-EX	67.6us		 9.3us

The measured latencies varied a bit from run-to-run. The wakeup
latency is much higher than the kicking latency.

A sample of statistics counts after system bootup (with vCPU
overcommit) was:

pv_hash_hops=1.00
pv_kick_unlock=1148
pv_kick_wake=1146
pv_latency_kick=11040
pv_latency_wake=194840
pv_spurious_wakeup=7
pv_wait_again=4
pv_wait_head=23
pv_wait_node=1129

Signed-off-by: Waiman Long <Waiman.Long@hpe.com>
---
 arch/x86/Kconfig                    |    8 +
 kernel/locking/qspinlock_paravirt.h |   32 ++++-
 kernel/locking/qspinlock_stat.h     |  291 +++++++++++++++++++++++++++++++++++
 3 files changed, 326 insertions(+), 5 deletions(-)
 create mode 100644 kernel/locking/qspinlock_stat.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4a9b9a9..403bfea 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -688,6 +688,14 @@ config PARAVIRT_SPINLOCKS
 
 	  If you are unsure how to answer this question, answer Y.
 
+config QUEUED_LOCK_STAT
+	bool "Paravirt queued spinlock statistics"
+	depends on PARAVIRT_SPINLOCKS && SYSFS && QUEUED_SPINLOCKS
+	---help---
+	  Enable the collection of statistical data on the slowpath
+	  behavior of paravirtualized queued spinlocks and report
+	  them on sysfs.
+
 source "arch/x86/xen/Kconfig"
 
 config KVM_GUEST
diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
index 4bd323d..aaeeefb 100644
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -41,6 +41,11 @@ struct pv_node {
 };
 
 /*
+ * Include queued spinlock statistics code
+ */
+#include "qspinlock_stat.h"
+
+/*
  * Lock and MCS node addresses hash table for fast lookup
  *
  * Hashing is done on a per-cacheline basis to minimize the need to access
@@ -100,10 +105,13 @@ static struct qspinlock **pv_hash(struct qspinlock *lock, struct pv_node *node)
 {
 	unsigned long offset, hash = hash_ptr(lock, pv_lock_hash_bits);
 	struct pv_hash_entry *he;
+	int hopcnt = 0;
 
 	for_each_hash_entry(he, offset, hash) {
+		hopcnt++;
 		if (!cmpxchg(&he->lock, NULL, lock)) {
 			WRITE_ONCE(he->node, node);
+			qstat_hop(hopcnt);
 			return &he->lock;
 		}
 	}
@@ -164,9 +172,11 @@ static void pv_init_node(struct mcs_spinlock *node)
 static void pv_wait_node(struct mcs_spinlock *node)
 {
 	struct pv_node *pn = (struct pv_node *)node;
+	int waitcnt = 0;
 	int loop;
 
-	for (;;) {
+	/* waitcnt processing will be compiled out if !QUEUED_LOCK_STAT */
+	for (;; waitcnt++) {
 		for (loop = SPIN_THRESHOLD; loop; loop--) {
 			if (READ_ONCE(node->locked))
 				return;
@@ -184,12 +194,16 @@ static void pv_wait_node(struct mcs_spinlock *node)
 		 */
 		smp_store_mb(pn->state, vcpu_halted);
 
-		if (!READ_ONCE(node->locked))
+		if (!READ_ONCE(node->locked)) {
+			qstat_inc(qstat_pv_wait_node, true);
+			qstat_inc(qstat_pv_wait_again, waitcnt);
 			pv_wait(&pn->state, vcpu_halted);
+		}
 
 		/*
-		 * If pv_kick_node() changed us to vcpu_hashed, retain that value
-		 * so that pv_wait_head() knows to not also try to hash this lock.
+		 * If pv_kick_node() changed us to vcpu_hashed, retain that
+		 * value so that pv_wait_head() knows to not also try to hash
+		 * this lock.
 		 */
 		cmpxchg(&pn->state, vcpu_halted, vcpu_running);
 
@@ -200,6 +214,7 @@ static void pv_wait_node(struct mcs_spinlock *node)
 		 * So it is better to spin for a while in the hope that the
 		 * MCS lock will be released soon.
 		 */
+		qstat_inc(qstat_pv_spurious_wakeup, !READ_ONCE(node->locked));
 	}
 
 	/*
@@ -250,6 +265,7 @@ static void pv_wait_head(struct qspinlock *lock, struct mcs_spinlock *node)
 	struct pv_node *pn = (struct pv_node *)node;
 	struct __qspinlock *l = (void *)lock;
 	struct qspinlock **lp = NULL;
+	int waitcnt = 0;
 	int loop;
 
 	/*
@@ -259,7 +275,7 @@ static void pv_wait_head(struct qspinlock *lock, struct mcs_spinlock *node)
 	if (READ_ONCE(pn->state) == vcpu_hashed)
 		lp = (struct qspinlock **)1;
 
-	for (;;) {
+	for (;; waitcnt++) {
 		for (loop = SPIN_THRESHOLD; loop; loop--) {
 			if (!READ_ONCE(l->locked))
 				return;
@@ -290,14 +306,19 @@ static void pv_wait_head(struct qspinlock *lock, struct mcs_spinlock *node)
 				return;
 			}
 		}
+		qstat_inc(qstat_pv_wait_head, true);
+		qstat_inc(qstat_pv_wait_again, waitcnt);
 		pv_wait(&l->locked, _Q_SLOW_VAL);
 
+		if (!READ_ONCE(l->locked))
+			return;
 		/*
 		 * The unlocker should have freed the lock before kicking the
 		 * CPU. So if the lock is still not free, it is a spurious
 		 * wakeup and so the vCPU should wait again after spinning for
 		 * a while.
 		 */
+		qstat_inc(qstat_pv_spurious_wakeup, true);
 	}
 
 	/*
@@ -352,6 +373,7 @@ __pv_queued_spin_unlock_slowpath(struct qspinlock *lock, u8 locked)
 	 * vCPU is harmless other than the additional latency in completing
 	 * the unlock.
 	 */
+	qstat_inc(qstat_pv_kick_unlock, true);
 	pv_kick(node->cpu);
 }
 
diff --git a/kernel/locking/qspinlock_stat.h b/kernel/locking/qspinlock_stat.h
new file mode 100644
index 0000000..16b84b2
--- /dev/null
+++ b/kernel/locking/qspinlock_stat.h
@@ -0,0 +1,291 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Authors: Waiman Long <waiman.long@hpe.com>
+ */
+
+/*
+ * When queued spinlock statistics is enabled, the following sysfs files
+ * will be created to hold the statistics counters:
+ *
+ * /sys/kernel/qlockstat/
+ *   pv_hash_hops	- average # of hops per hashing operation
+ *   pv_kick_unlock	- # of vCPU kicks issued at unlock time
+ *   pv_kick_wake	- # of vCPU kicks used for computing pv_latency_wake
+ *   pv_latency_kick	- average latency (ns) of vCPU kick operation
+ *   pv_latency_wake	- average latency (ns) from vCPU kick to wakeup
+ *   pv_spurious_wakeup	- # of spurious wakeups
+ *   pv_wait_again	- # of vCPU wait's that happened after a vCPU kick
+ *   pv_wait_head	- # of vCPU wait's at the queue head
+ *   pv_wait_node	- # of vCPU wait's at a non-head queue node
+ *
+ * Writing to the "reset_counters" file will reset all the above counter
+ * values.
+ *
+ * These statistics counters are implemented as per-cpu variables which are
+ * summed and computed whenever the corresponding sysfs files are read. This
+ * minimizes added overhead making the counters usable even in a production
+ * environment.
+ *
+ * There may be slight difference between pv_kick_wake and pv_kick_unlock.
+ */
+enum qlock_stats {
+	qstat_pv_hash_hops,
+	qstat_pv_kick_unlock,
+	qstat_pv_kick_wake,
+	qstat_pv_latency_kick,
+	qstat_pv_latency_wake,
+	qstat_pv_spurious_wakeup,
+	qstat_pv_wait_again,
+	qstat_pv_wait_head,
+	qstat_pv_wait_node,
+	qstat_num,	/* Total number of statistics counters */
+	qstat_reset_cnts = qstat_num,
+};
+
+#ifdef CONFIG_QUEUED_LOCK_STAT
+/*
+ * Collect pvqspinlock statistics
+ */
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/sched.h>
+
+static const char * const qstat_names[qstat_num + 1] = {
+	[qstat_pv_hash_hops]	   = "pv_hash_hops",
+	[qstat_pv_kick_unlock]     = "pv_kick_unlock",
+	[qstat_pv_kick_wake]       = "pv_kick_wake",
+	[qstat_pv_spurious_wakeup] = "pv_spurious_wakeup",
+	[qstat_pv_latency_kick]	   = "pv_latency_kick",
+	[qstat_pv_latency_wake]    = "pv_latency_wake",
+	[qstat_pv_wait_again]      = "pv_wait_again",
+	[qstat_pv_wait_head]       = "pv_wait_head",
+	[qstat_pv_wait_node]       = "pv_wait_node",
+	[qstat_reset_cnts]         = "reset_counters",
+};
+
+/*
+ * Per-cpu counters
+ */
+static DEFINE_PER_CPU(unsigned long, qstats[qstat_num]);
+static DEFINE_PER_CPU(u64, pv_kick_time);
+
+/*
+ * Sysfs data structures
+ */
+static struct kobj_attribute qstat_kobj_attrs[qstat_num + 1];
+static struct attribute *attrs[qstat_num + 2];
+static struct kobject *qstat_kobj;
+static struct attribute_group attr_group = {
+	.attrs = attrs,
+};
+
+/*
+ * Function to show the qlock statistics count
+ */
+static ssize_t
+qstat_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	int cpu, idx;
+	u64 stat = 0;
+
+	/*
+	 * Compute the index of the kobj_attribute in the array and used
+	 * it as the same index as the per-cpu variable
+	 */
+	idx = attr - qstat_kobj_attrs;
+
+	for_each_online_cpu(cpu)
+		stat += per_cpu(qstats[idx], cpu);
+	return sprintf(buf, "%llu\n", stat);
+}
+
+/*
+ * Return the average kick latency (ns) = pv_latency_kick/pv_kick_unlock
+ */
+static ssize_t
+kick_latency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	int cpu;
+	u64 latencies = 0, kicks = 0;
+
+	for_each_online_cpu(cpu) {
+		kicks     += per_cpu(qstats[qstat_pv_kick_unlock],  cpu);
+		latencies += per_cpu(qstats[qstat_pv_latency_kick], cpu);
+	}
+
+	/* Rounded to the nearest ns */
+	return sprintf(buf, "%llu\n", kicks ? (latencies + kicks/2)/kicks : 0);
+}
+
+/*
+ * Return the average wake latency (ns) = pv_latency_wake/pv_kick_wake
+ */
+static ssize_t
+wake_latency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	int cpu;
+	u64 latencies = 0, kicks = 0;
+
+	for_each_online_cpu(cpu) {
+		kicks     += per_cpu(qstats[qstat_pv_kick_wake],    cpu);
+		latencies += per_cpu(qstats[qstat_pv_latency_wake], cpu);
+	}
+
+	/* Rounded to the nearest ns */
+	return sprintf(buf, "%llu\n", kicks ? (latencies + kicks/2)/kicks : 0);
+}
+
+/*
+ * Return the average hops/hash = pv_hash_hops/pv_kick_unlock
+ */
+static ssize_t
+hash_hop_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	int cpu;
+	u64 hops = 0, kicks = 0;
+
+	for_each_online_cpu(cpu) {
+		kicks += per_cpu(qstats[qstat_pv_kick_unlock], cpu);
+		hops  += per_cpu(qstats[qstat_pv_hash_hops],   cpu);
+	}
+
+	if (!kicks)
+		return sprintf(buf, "0\n");
+
+	/*
+	 * Return a X.XX decimal number
+	 */
+	return sprintf(buf, "%llu.%02llu\n", hops/kicks,
+		      ((hops%kicks)*100 + kicks/2)/kicks);
+}
+
+/*
+ * Reset all the counters value
+ *
+ * Since the counter updates aren't atomic, the resetting is done twice
+ * to make sure that the counters are very likely to be all cleared.
+ */
+static ssize_t
+reset_counters_store(struct kobject *kobj, struct kobj_attribute *attr,
+		     const char *buf, size_t count)
+{
+	int cpu;
+
+	for_each_online_cpu(cpu) {
+		int i;
+		unsigned long *ptr = per_cpu_ptr(qstats, cpu);
+
+		for (i = 0 ; i < qstat_num; i++)
+			WRITE_ONCE(ptr[i], 0);
+		for (i = 0 ; i < qstat_num; i++)
+			WRITE_ONCE(ptr[i], 0);
+	}
+	return count;
+}
+
+/*
+ * Initialize sysfs for the qspinlock statistics
+ */
+static int __init init_qspinlock_stat(void)
+{
+	int i, retval;
+
+	qstat_kobj = kobject_create_and_add("qlockstat", kernel_kobj);
+	if (qstat_kobj == NULL)
+		return -ENOMEM;
+
+	/*
+	 * Initialize the attribute table
+	 *
+	 * As reading from and writing to the stat files can be slow, only
+	 * root is allowed to do the read/write to limit impact to system
+	 * performance.
+	 */
+	for (i = 0; i <= qstat_num; i++) {
+		qstat_kobj_attrs[i].attr.name = qstat_names[i];
+		qstat_kobj_attrs[i].attr.mode = 0400;
+		qstat_kobj_attrs[i].show      = qstat_show;
+		attrs[i]		      = &qstat_kobj_attrs[i].attr;
+	}
+	qstat_kobj_attrs[qstat_pv_hash_hops].show    = hash_hop_show;
+	qstat_kobj_attrs[qstat_pv_latency_kick].show = kick_latency_show;
+	qstat_kobj_attrs[qstat_pv_latency_wake].show = wake_latency_show;
+
+	/*
+	 * Set attributes for reset_counters
+	 */
+	qstat_kobj_attrs[qstat_reset_cnts].attr.mode = 0200;
+	qstat_kobj_attrs[qstat_reset_cnts].show      = NULL;
+	qstat_kobj_attrs[qstat_reset_cnts].store     = reset_counters_store;
+
+	retval = sysfs_create_group(qstat_kobj, &attr_group);
+	if (retval)
+		kobject_put(qstat_kobj);
+
+	return retval;
+}
+fs_initcall(init_qspinlock_stat);
+
+/*
+ * Increment the PV qspinlock statistics counters
+ */
+static inline void qstat_inc(enum qlock_stats stat, bool cond)
+{
+	if (cond)
+		this_cpu_inc(qstats[stat]);
+}
+
+/*
+ * PV hash hop count
+ */
+static inline void qstat_hop(int hopcnt)
+{
+	this_cpu_add(qstats[qstat_pv_hash_hops], hopcnt);
+}
+
+/*
+ * Replacement function for pv_kick()
+ */
+static inline void __pv_kick(int cpu)
+{
+	u64 start = sched_clock();
+
+	per_cpu(pv_kick_time, cpu) = start;
+	pv_kick(cpu);
+	this_cpu_add(qstats[qstat_pv_latency_kick], sched_clock() - start);
+}
+
+/*
+ * Replacement function for pv_wait()
+ */
+static inline void __pv_wait(u8 *ptr, u8 val)
+{
+	u64 *pkick_time = this_cpu_ptr(&pv_kick_time);
+
+	*pkick_time = 0;
+	pv_wait(ptr, val);
+	if (*pkick_time) {
+		this_cpu_add(qstats[qstat_pv_latency_wake],
+			     sched_clock() - *pkick_time);
+		qstat_inc(qstat_pv_kick_wake, true);
+	}
+}
+
+#define pv_kick(c)	__pv_kick(c)
+#define pv_wait(p, v)	__pv_wait(p, v)
+
+#else /* CONFIG_QUEUED_LOCK_STAT */
+
+static inline void qstat_inc(enum qlock_stats stat, bool cond)	{ }
+static inline void qstat_hop(int hopcnt)			{ }
+
+#endif /* CONFIG_QUEUED_LOCK_STAT */
-- 
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] | [prev] | [next] | [standalone]

#1260788 — Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect slowpath lock statistics

On Fri, Oct 30, 2015 at 07:26:35PM -0400, Waiman Long wrote:
> This patch enables the accumulation of kicking and waiting related
> PV qspinlock statistics when the new QUEUED_LOCK_STAT configuration
> option is selected. It also enables the collection of data which
> enable us to calculate the kicking and wakeup latencies which have
> a heavy dependency on the CPUs being used.
> 
> The statistical counters are per-cpu variables to minimize the
> performance overhead in their updates. These counters are exported
> via the sysfs filesystem under the /sys/kernel/qlockstat directory.
> When the corresponding sysfs files are read, summation and computing
> of the required data are then performed.

Why did you switch to sysfs? You can create custom debugfs files too.

> @@ -259,7 +275,7 @@ static void pv_wait_head(struct qspinlock *lock, struct mcs_spinlock *node)
>  	if (READ_ONCE(pn->state) == vcpu_hashed)
>  		lp = (struct qspinlock **)1;
>  
> -	for (;;) {
> +	for (;; waitcnt++) {
>  		for (loop = SPIN_THRESHOLD; loop; loop--) {
>  			if (!READ_ONCE(l->locked))
>  				return;

Did you check that goes away when !STAT ?

> +/*
> + * Return the average kick latency (ns) = pv_latency_kick/pv_kick_unlock
> + */
> +static ssize_t
> +kick_latency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> +	int cpu;
> +	u64 latencies = 0, kicks = 0;
> +
> +	for_each_online_cpu(cpu) {

I think you need for_each_possible_cpu(), otherwise the results will
change with hotplug operations.

> +		kicks     += per_cpu(qstats[qstat_pv_kick_unlock],  cpu);
> +		latencies += per_cpu(qstats[qstat_pv_latency_kick], cpu);
> +	}
> +
> +	/* Rounded to the nearest ns */
> +	return sprintf(buf, "%llu\n", kicks ? (latencies + kicks/2)/kicks : 0);
> +}
--
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]

#1263397 — Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect slowpath lock statistics

On 11/02/2015 11:40 AM, Peter Zijlstra wrote:
> On Fri, Oct 30, 2015 at 07:26:35PM -0400, Waiman Long wrote:
>> This patch enables the accumulation of kicking and waiting related
>> PV qspinlock statistics when the new QUEUED_LOCK_STAT configuration
>> option is selected. It also enables the collection of data which
>> enable us to calculate the kicking and wakeup latencies which have
>> a heavy dependency on the CPUs being used.
>>
>> The statistical counters are per-cpu variables to minimize the
>> performance overhead in their updates. These counters are exported
>> via the sysfs filesystem under the /sys/kernel/qlockstat directory.
>> When the corresponding sysfs files are read, summation and computing
>> of the required data are then performed.
> Why did you switch to sysfs? You can create custom debugfs files too.

I was not aware of that capability. So you mean using 
debugfs_create_file() using custom file_operations. Right? That doesn't 
seem to be easier than using sysfs. However, I can use that if you think 
it is better to use debugfs.

>
>> @@ -259,7 +275,7 @@ static void pv_wait_head(struct qspinlock *lock, struct mcs_spinlock *node)
>>   	if (READ_ONCE(pn->state) == vcpu_hashed)
>>   		lp = (struct qspinlock **)1;
>>
>> -	for (;;) {
>> +	for (;; waitcnt++) {
>>   		for (loop = SPIN_THRESHOLD; loop; loop--) {
>>   			if (!READ_ONCE(l->locked))
>>   				return;
> Did you check that goes away when !STAT ?

Yes, the increment code goes away when !STAT. I had added a comment to 
talk about that.

>
>> +/*
>> + * Return the average kick latency (ns) = pv_latency_kick/pv_kick_unlock
>> + */
>> +static ssize_t
>> +kick_latency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>> +{
>> +	int cpu;
>> +	u64 latencies = 0, kicks = 0;
>> +
>> +	for_each_online_cpu(cpu) {
> I think you need for_each_possible_cpu(), otherwise the results will
> change with hotplug operations.

Right, I will make the change.

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]

#1263413 — Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect slowpath lock statistics

On Thu, Nov 05, 2015 at 11:29:29AM -0500, Waiman Long wrote:
> On 11/02/2015 11:40 AM, Peter Zijlstra wrote:
> >On Fri, Oct 30, 2015 at 07:26:35PM -0400, Waiman Long wrote:
> >>This patch enables the accumulation of kicking and waiting related
> >>PV qspinlock statistics when the new QUEUED_LOCK_STAT configuration
> >>option is selected. It also enables the collection of data which
> >>enable us to calculate the kicking and wakeup latencies which have
> >>a heavy dependency on the CPUs being used.
> >>
> >>The statistical counters are per-cpu variables to minimize the
> >>performance overhead in their updates. These counters are exported
> >>via the sysfs filesystem under the /sys/kernel/qlockstat directory.
> >>When the corresponding sysfs files are read, summation and computing
> >>of the required data are then performed.
> >Why did you switch to sysfs? You can create custom debugfs files too.
> 
> I was not aware of that capability. So you mean using debugfs_create_file()
> using custom file_operations. Right? 

Yep.

> That doesn't seem to be easier than
> using sysfs. However, I can use that if you think it is better to use
> debugfs.

Mostly I just wanted to point out that it was possible; you need not
change to sysfs because debugfs lacks the capability.

But now that you ask, I think debugfs might be the better place, such
statistics (and the proposed CONFIG symbol) are purely for debug
purposes, right?
--
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]

#1263428 — Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect slowpath lock statistics

On 11/05/2015 11:43 AM, Peter Zijlstra wrote:
> On Thu, Nov 05, 2015 at 11:29:29AM -0500, Waiman Long wrote:
>> On 11/02/2015 11:40 AM, Peter Zijlstra wrote:
>>> On Fri, Oct 30, 2015 at 07:26:35PM -0400, Waiman Long wrote:
>>>> This patch enables the accumulation of kicking and waiting related
>>>> PV qspinlock statistics when the new QUEUED_LOCK_STAT configuration
>>>> option is selected. It also enables the collection of data which
>>>> enable us to calculate the kicking and wakeup latencies which have
>>>> a heavy dependency on the CPUs being used.
>>>>
>>>> The statistical counters are per-cpu variables to minimize the
>>>> performance overhead in their updates. These counters are exported
>>>> via the sysfs filesystem under the /sys/kernel/qlockstat directory.
>>>> When the corresponding sysfs files are read, summation and computing
>>>> of the required data are then performed.
>>> Why did you switch to sysfs? You can create custom debugfs files too.
>> I was not aware of that capability. So you mean using debugfs_create_file()
>> using custom file_operations. Right?
> Yep.
>
>> That doesn't seem to be easier than
>> using sysfs. However, I can use that if you think it is better to use
>> debugfs.
> Mostly I just wanted to point out that it was possible; you need not
> change to sysfs because debugfs lacks the capability.
>
> But now that you ask, I think debugfs might be the better place, such
> statistics (and the proposed CONFIG symbol) are purely for debug
> purposes, right?

Davidlohr had asked me to use per-cpu counters to reduce performance 
overhead so that they can be usable in production system. That is 
another reason why I move to sysfs.

BTW, do you have comments on the other patches in the series? I would 
like to collect all the comments before I renew the series.

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]

#1263430 — Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect slowpath lock statistics

On Thu, Nov 05, 2015 at 11:59:21AM -0500, Waiman Long wrote:
> >Mostly I just wanted to point out that it was possible; you need not
> >change to sysfs because debugfs lacks the capability.
> >
> >But now that you ask, I think debugfs might be the better place, such
> >statistics (and the proposed CONFIG symbol) are purely for debug
> >purposes, right?
> 
> Davidlohr had asked me to use per-cpu counters to reduce performance
> overhead so that they can be usable in production system. That is another
> reason why I move to sysfs.

Yes, the per-cpu thing certainly makes sense. But as said, that does not
require you to move to sysfs.

> BTW, do you have comments on the other patches in the series? I would like
> to collect all the comments before I renew the series.

I still have to look at the last two patches, I've sadly not had time
for that yet.
--
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]

#1263447 — Re: [PATCH tip/locking/core v9 4/6] locking/pvqspinlock: Collect slowpath lock statistics

On 11/05/2015 12:09 PM, Peter Zijlstra wrote:
> On Thu, Nov 05, 2015 at 11:59:21AM -0500, Waiman Long wrote:
>>> Mostly I just wanted to point out that it was possible; you need not
>>> change to sysfs because debugfs lacks the capability.
>>>
>>> But now that you ask, I think debugfs might be the better place, such
>>> statistics (and the proposed CONFIG symbol) are purely for debug
>>> purposes, right?
>> Davidlohr had asked me to use per-cpu counters to reduce performance
>> overhead so that they can be usable in production system. That is another
>> reason why I move to sysfs.
> Yes, the per-cpu thing certainly makes sense. But as said, that does not
> require you to move to sysfs.
>
>> BTW, do you have comments on the other patches in the series? I would like
>> to collect all the comments before I renew the series.
> I still have to look at the last two patches, I've sadly not had time
> for that yet.

That is what I thought. Just let me know when you are done with the 
review, and I will update the patches and send out a new series.

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]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web