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 4/6] locking/pvqspinlock: Allow vCPUs kick-ahead
Date: Thu, 16 Jul 2015 10:51:50 -0400 [thread overview]
Message-ID: <55A7C506.9030309@hp.com> (raw)
In-Reply-To: <20150716054626.GV19282@twins.programming.kicks-ass.net>
On 07/16/2015 01:46 AM, Peter Zijlstra wrote:
> On Wed, Jul 15, 2015 at 10:01:02PM -0400, Waiman Long wrote:
>> On 07/15/2015 05:39 AM, Peter Zijlstra wrote:
>>> On Tue, Jul 14, 2015 at 10:13:35PM -0400, Waiman Long wrote:
>>>> Frequent CPU halting (vmexit) and CPU kicking (vmenter) lengthens
>>>> critical section and block forward progress. This patch implements
>>>> a kick-ahead mechanism where the unlocker will kick the queue head
>>>> vCPUs as well as up to four additional vCPUs next to the queue head
>>>> if they were halted. The kickings are done after exiting the critical
>>>> section to improve parallelism.
>>>>
>>>> The amount of kick-ahead allowed depends on the number of vCPUs
>>>> in the VM guest. This patch, by itself, won't do much as most of
>>>> the kickings are currently done at lock time. Coupled with the next
>>>> patch that defers lock time kicking to unlock time, it should improve
>>>> overall system performance in a busy overcommitted guest.
>>>>
>>>> Linux kernel builds were run in KVM guest on an 8-socket, 4
>>>> cores/socket Westmere-EX system and a 4-socket, 8 cores/socket
>>>> Haswell-EX system. Both systems are configured to have 32 physical
>>>> CPUs. The kernel build times before and after the patch were:
>>>>
>>>> Westmere Haswell
>>>> Patch 32 vCPUs 48 vCPUs 32 vCPUs 48 vCPUs
>>>> ----- -------- -------- -------- --------
>>>> Before patch 3m25.0s 10m34.1s 2m02.0s 15m35.9s
>>>> After patch 3m27.4s 10m32.0s 2m00.8s 14m52.5s
>>>>
>>>> There wasn't too much difference before and after the patch.
>>> That means either the patch isn't worth it, or as you seem to imply its
>>> in the wrong place in this series.
>> It needs to be coupled with the next patch to be effective as most of the
>> kicking are happening at the lock side, instead of at the unlock side. If
>> you look at the sample pvqspinlock stats in patch 3:
>>
>> lock_kick_count=755354
>> unlock_kick_count=87
>>
>> The number of unlock kicks is negligible compared with the lock kicks. Patch
>> 5 does have a dependency on patch 4 unless we make it unconditionally defers
>> kicking to the unlock call which was what I had done in the v1 patch. The
>> reason why I change this in v2 is because I found a very slight performance
>> degradation in doing so.
> This way we cannot see the gains of the proposed complexity. So put it
> in a place where you can.
OK, I will see what I can do to make the performance change more visible
on a patch-by-patch basis.
>>> You also do not offer any support for any of the magic numbers..
>> I chose 4 for PV_KICK_AHEAD_MAX as I didn't see much performance difference
>> when I did a kick-ahead of 5. Also, it may be too unfair to the vCPU that
>> was doing the kicking if the number is too big. Another magic number is
>> pv_kick_ahead number. This one is kind of arbitrary. Right now I do a log2,
>> but it can be divided by 4 (rshift 2) as well.
> So what was the difference between 1-2-3-4 ? I would be thinking one
> extra kick is the biggest help, no?
I was seeing diminishing returns with more kicks. I can add a table on
that in the next patch.
Cheers,
Longman
next prev parent reply other threads:[~2015-07-16 14:51 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 [this message]
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
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=55A7C506.9030309@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.