From: Wanpeng Li <kernellwp@gmail.com>
To: Ingo Molnar <mingo@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc: Wanpeng Li <wanpeng.li@hotmail.com>,
Waiman Long <Waiman.Long@hpe.com>,
Davidlohr Bueso <dave@stgolabs.net>
Subject: [PATCH RESEND v4] locking/pvqspinlock: Fix double hash race
Date: Wed, 27 Jul 2016 19:30:37 +0800 [thread overview]
Message-ID: <1469619037-13826-1-git-send-email-wanpeng.li@hotmail.com> (raw)
From: Wanpeng Li <wanpeng.li@hotmail.com>
When the lock holder vCPU is racing with the queue head vCPU:
lock holder vCPU queue head vCPU
===================== ==================
node->locked = 1;
<preemption> READ_ONCE(node->locked)
... pv_wait_head_or_lock():
SPIN_THRESHOLD loop;
pv_hash();
lock->locked = _Q_SLOW_VAL;
node->state = vcpu_hashed;
pv_kick_node():
cmpxchg(node->state,
vcpu_halted, vcpu_hashed);
lock->locked = _Q_SLOW_VAL;
pv_hash();
With preemption at the right moment, it is possible that both the
lock holder and queue head vCPUs can be racing to set node->state
which can result in hash entry race. Making sure the state is never
set to vcpu_halted will prevent this racing from happening.
This patch fix it by setting vcpu_hashed after we did all hash thing.
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Waiman Long <Waiman.Long@hpe.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
v3 -> v4:
* update patch subject
* add code comments
v2 -> v3:
* fix typo in patch description
v1 -> v2:
* adjust patch description
kernel/locking/qspinlock_paravirt.h | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
index 21ede57..ca96db4 100644
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -450,7 +450,28 @@ pv_wait_head_or_lock(struct qspinlock *lock, struct mcs_spinlock *node)
goto gotlock;
}
}
- WRITE_ONCE(pn->state, vcpu_halted);
+ /*
+ * lock holder vCPU queue head vCPU
+ * ---------------- ---------------
+ * node->locked = 1;
+ * <preemption> READ_ONCE(node->locked)
+ * ... pv_wait_head_or_lock():
+ * SPIN_THRESHOLD loop;
+ * pv_hash();
+ * lock->locked = _Q_SLOW_VAL;
+ * node->state = vcpu_hashed;
+ * pv_kick_node():
+ * cmpxchg(node->state,
+ * vcpu_halted, vcpu_hashed);
+ * lock->locked = _Q_SLOW_VAL;
+ * pv_hash();
+ *
+ * With preemption at the right moment, it is possible that both the
+ * lock holder and queue head vCPUs can be racing to set node->state.
+ * Making sure the state is never set to vcpu_halted will prevent this
+ * racing from happening.
+ */
+ WRITE_ONCE(pn->state, vcpu_hashed);
qstat_inc(qstat_pv_wait_head, true);
qstat_inc(qstat_pv_wait_again, waitcnt);
pv_wait(&l->locked, _Q_SLOW_VAL);
--
2.1.0
next reply other threads:[~2016-07-27 11:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-27 11:30 Wanpeng Li [this message]
2016-08-03 3:28 ` [PATCH RESEND v4] locking/pvqspinlock: Fix double hash race Waiman Long
2016-08-03 8:10 ` Wanpeng Li
2016-08-09 9:37 Wanpeng Li
2016-08-09 10:49 ` Peter Zijlstra
2016-08-09 11:31 ` Wanpeng Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1469619037-13826-1-git-send-email-wanpeng.li@hotmail.com \
--to=kernellwp@gmail.com \
--cc=Waiman.Long@hpe.com \
--cc=dave@stgolabs.net \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=wanpeng.li@hotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.