From: Waiman Long <waiman.long@hp.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
"H. Peter Anvin" <hpa@zytor.com>,
x86@kernel.org, linux-kernel@vger.kernel.org,
Scott J Norton <scott.norton@hp.com>,
Douglas Hatch <doug.hatch@hp.com>,
Davidlohr Bueso <dave@stgolabs.net>
Subject: Re: [PATCH v2 5/6] locking/pvqspinlock: Opportunistically defer kicking to unlock time
Date: Wed, 15 Jul 2015 22:18:35 -0400 [thread overview]
Message-ID: <55A7147B.8020406@hp.com> (raw)
In-Reply-To: <20150715100309.GJ2859@worktop.programming.kicks-ass.net>
On 07/15/2015 06:03 AM, Peter Zijlstra wrote:
> On Tue, Jul 14, 2015 at 10:13:36PM -0400, Waiman Long wrote:
>> +static void pv_kick_node(struct qspinlock *lock, struct mcs_spinlock *node)
>> {
>> struct pv_node *pn = (struct pv_node *)node;
>>
>> + if (xchg(&pn->state, vcpu_running) == vcpu_running)
>> + return;
>> +
>> /*
>> + * Kicking the next node at lock time can actually be a bit faster
>> + * than doing it at unlock time because the critical section time
>> + * overlaps with the wakeup latency of the next node. However, if the
>> + * VM is too overcommmitted, it can happen that we need to kick the
>> + * CPU again at unlock time (double-kick). To avoid that and also to
>> + * fully utilize the kick-ahead functionality at unlock time,
>> + * the kicking will be deferred under either one of the following
>> + * 2 conditions:
>> *
>> + * 1) The VM guest has too few vCPUs that kick-ahead is not even
>> + * enabled. In this case, the chance of double-kick will be
>> + * higher.
>> + * 2) The node after the next one is also in the halted state.
>> *
>> + * In this case, the hashed flag is set to indicate that hashed
>> + * table has been filled and _Q_SLOW_VAL is set.
>> */
>> - if (xchg(&pn->state, vcpu_running) == vcpu_halted) {
>> - pvstat_inc(pvstat_lock_kick);
>> - pv_kick(pn->cpu);
>> + if ((!pv_kick_ahead || pv_get_kick_node(pn, 1))&&
>> + (xchg(&pn->hashed, 1) == 0)) {
>> + struct __qspinlock *l = (void *)lock;
>> +
>> + /*
>> + * As this is the same vCPU that will check the _Q_SLOW_VAL
>> + * value and the hash table later on at unlock time, no atomic
>> + * instruction is needed.
>> + */
>> + WRITE_ONCE(l->locked, _Q_SLOW_VAL);
>> + (void)pv_hash(lock, pn);
>> + return;
>> }
>> +
>> + /*
>> + * Kicking the vCPU even if it is not really halted is safe.
>> + */
>> + pvstat_inc(pvstat_lock_kick);
>> + pv_kick(pn->cpu);
>> }
>>
>> /*
>> @@ -513,6 +545,13 @@ static void pv_wait_head(struct qspinlock *lock, struct mcs_spinlock *node)
>> cpu_relax();
>> }
>>
>> + if (!lp&& (xchg(&pn->hashed, 1) == 1))
>> + /*
>> + * The hashed table& _Q_SLOW_VAL had been filled
>> + * by the lock holder.
>> + */
>> + lp = (struct qspinlock **)-1;
>> +
>> if (!lp) { /* ONCE */
>> lp = pv_hash(lock, pn);
>> /*
> *groan*, so you complained the previous version of this patch was too
> complex, but let me say I vastly preferred it to this one :/
I said it was complex as maintaining a tri-state variable needed more
thought than 2 bi-state variables. I can revert it back to the tri-state
variable as doing an unconditional kick in unlock simplifies the code at
pv_wait_head().
Cheers,
Longman
next prev parent reply other threads:[~2015-07-16 2:18 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-15 2:13 [PATCH 0/6 v2] locking/qspinlock: Enhance pvqspinlock performance Waiman Long
2015-07-15 2:13 ` [PATCH v2 1/6] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW_VAL Waiman Long
2015-07-15 9:10 ` Peter Zijlstra
2015-07-16 0:18 ` Waiman Long
2015-07-16 5:42 ` Peter Zijlstra
2015-07-16 14:07 ` Waiman Long
2015-07-16 15:04 ` Waiman Long
2015-07-16 15:10 ` Will Deacon
2015-08-03 16:59 ` [tip:locking/core] locking/Documentation: Clarify failed cmpxchg( ) memory ordering semantics tip-bot for Will Deacon
2015-08-03 17:36 ` Davidlohr Bueso
2015-07-15 2:13 ` [PATCH v2 2/6] locking/pvqspinlock: Add pending bit support Waiman Long
2015-07-15 2:13 ` [PATCH v2 3/6] locking/pvqspinlock: Collect slowpath lock statistics Waiman Long
2015-07-15 2:13 ` [PATCH v2 4/6] locking/pvqspinlock: Allow vCPUs kick-ahead Waiman Long
2015-07-15 9:39 ` Peter Zijlstra
2015-07-16 2:01 ` Waiman Long
2015-07-16 5:46 ` Peter Zijlstra
2015-07-16 14:51 ` Waiman Long
2015-07-15 2:13 ` [PATCH v2 5/6] locking/pvqspinlock: Opportunistically defer kicking to unlock time Waiman Long
2015-07-15 6:14 ` Raghavendra K T
2015-07-15 10:03 ` Peter Zijlstra
2015-07-16 2:18 ` Waiman Long [this message]
2015-07-16 5:49 ` Peter Zijlstra
2015-07-15 2:13 ` [PATCH v2 6/6] locking/pvqspinlock: Queue node adaptive spinning Waiman Long
2015-07-15 10:01 ` Peter Zijlstra
2015-07-16 2:13 ` Waiman Long
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=55A7147B.8020406@hp.com \
--to=waiman.long@hp.com \
--cc=dave@stgolabs.net \
--cc=doug.hatch@hp.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=scott.norton@hp.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/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.