Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755345AbbGOJjp (ORCPT ); Wed, 15 Jul 2015 05:39:45 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:41370 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755317AbbGOJja (ORCPT ); Wed, 15 Jul 2015 05:39:30 -0400 Date: Wed, 15 Jul 2015 11:39:24 +0200 From: Peter Zijlstra To: Waiman Long Cc: Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Scott J Norton , Douglas Hatch , Davidlohr Bueso Subject: Re: [PATCH v2 4/6] locking/pvqspinlock: Allow vCPUs kick-ahead Message-ID: <20150715093924.GH2859@worktop.programming.kicks-ass.net> References: <1436926417-20256-1-git-send-email-Waiman.Long@hp.com> <1436926417-20256-5-git-send-email-Waiman.Long@hp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1436926417-20256-5-git-send-email-Waiman.Long@hp.com> User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2775 Lines: 75 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. > @@ -224,7 +233,16 @@ static unsigned int pv_lock_hash_bits __read_mostly; > */ > void __init __pv_init_lock_hash(void) > { > - int pv_hash_size = ALIGN(4 * num_possible_cpus(), PV_HE_PER_LINE); > + int ncpus = num_possible_cpus(); > + int pv_hash_size = ALIGN(4 * ncpus, PV_HE_PER_LINE); > + int i; > + > + /* > + * The minimum number of vCPUs required in each kick-ahead level > + */ > + static const u8 kick_ahead_threshold[PV_KICK_AHEAD_MAX] = { > + 4, 8, 16, 32 > + }; You are aware we have ilog2(), right? > + /* > + * Enable the unlock kick ahead mode according to the number of > + * vCPUs available. > + */ > + for (i = PV_KICK_AHEAD_MAX; i > 0; i--) > + if (ncpus >= kick_ahead_threshold[i - 1]) { > + pv_kick_ahead = i; > + break; > + } That's missing { }. > + if (pv_kick_ahead) > + pr_info("PV unlock kick ahead level %d enabled\n", > + pv_kick_ahead); Idem. That said, I still really dislike this patch, it again seems a random bunch of hacks. You also do not offer any support for any of the magic numbers.. -- 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/