From patchwork Wed Oct 12 16:50:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tim Chen X-Patchwork-Id: 9373585 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 460C960839 for ; Wed, 12 Oct 2016 16:53:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 364D8296FF for ; Wed, 12 Oct 2016 16:53:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 29FC729706; Wed, 12 Oct 2016 16:53:56 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6DDE729700 for ; Wed, 12 Oct 2016 16:53:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753716AbcJLQxx (ORCPT ); Wed, 12 Oct 2016 12:53:53 -0400 Received: from mga11.intel.com ([192.55.52.93]:24006 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755789AbcJLQxu (ORCPT ); Wed, 12 Oct 2016 12:53:50 -0400 Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 12 Oct 2016 09:50:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,336,1473145200"; d="scan'208";a="771827915" Received: from schen9-mobl.jf.intel.com (HELO linux.intel.com) ([10.23.236.155]) by FMSMGA003.fm.intel.com with ESMTP; 12 Oct 2016 09:50:43 -0700 Date: Wed, 12 Oct 2016 09:50:42 -0700 From: Tim Chen To: Thomas Gleixner Cc: Srinivas Pandruvada , rjw@rjwysocki.net, mingo@redhat.com, bp@suse.de, x86@kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, peterz@infradead.org, jolsa@redhat.com Subject: Re: [PATCH v5 5/9] x86/sysctl: Add sysctl for ITMT scheduling feature Message-ID: <20161012165042.GA24463@linux.intel.com> Reply-To: tim.c.chen@linux.intel.com References: <1475322326-160112-1-git-send-email-srinivas.pandruvada@linux.intel.com> <1475322326-160112-6-git-send-email-srinivas.pandruvada@linux.intel.com> <1475684673.3916.299.camel@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.0 (2016-08-17) Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Oct 06, 2016 at 01:13:08PM +0200, Thomas Gleixner wrote: > On Wed, 5 Oct 2016, Tim Chen wrote: > > On Wed, 2016-10-05 at 16:35 +0200, Thomas Gleixner wrote: > > > > + if (itmt_supported) { > > > > + itmt_sysctl_header = > > > > + register_sysctl_table(itmt_root_table); > > > > + if (!itmt_sysctl_header) { > > > > + mutex_unlock(&itmt_update_mutex); > > > > + return; > > > So you now have a state of capable which cannot be enabled. Whats the > > > point? > > > > For multi-socket system where ITMT is not enabled by default, the operator > > can still decide to enable it via sysctl. > > With a sysctl which failed to be installed. Good luck with that. > > > > > + } > > > > + /* > > > > +  * ITMT capability automatically enables ITMT > > > > +  * scheduling for small systems (single node). > > > > +  */ > > > > + if (topology_num_packages() == 1) > > > > + sysctl_sched_itmt_enabled = 1; > > > > + } else { > > > > + if (itmt_sysctl_header) > > > > + unregister_sysctl_table(itmt_sysctl_header); > > > > + } > > > > + > > > > + if (sysctl_sched_itmt_enabled) { > > > > + /* disable sched_itmt if we are no longer ITMT capable */ > > > > + if (!itmt_supported) > > > > > > How do you get here if itmt is not supported?  > > > > If the OS decides to turn off ITMT for any reason, (i.e. invoke  > > sched_set_itmt_support(false) after it has turned on itmt_support > > before), this is the logic to do it.  We don't turn off ITMT support > > after it has been turned on today, in the future the OS may. > > Then please make this two functions (set/clear) so one can actually follow > the logic. The above is just too convoluted. > Thomas, Will the update patch below address your concerns for this patch? Please let us know if you have any other additional comments about this series. We'll like to address all of them before we post an update to the series. Thanks. Tim --->8--- From: Tim Chen Subject: [PATCH 5/6 v5 - update proposal] x86/sysctl: Add sysctl for ITMT scheduling feature Intel Turbo Boost Max Technology 3.0 (ITMT) feature allows some cores to be boosted to higher turbo frequency than others. Add /proc/sys/kernel/sched_itmt_enabled so operator can enable/disable scheduling of tasks that favor cores with higher turbo boost frequency potential. By default, system that is ITMT capable and single socket has this feature turned on. It is more likely to be lightly loaded and operates in Turbo range. When there is a change in the ITMT scheduling operation desired, a rebuild of the sched domain is initiated so the scheduler can set up sched domains with appropriate flag to enable/disable ITMT scheduling operations. Signed-off-by: Tim Chen Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Srinivas Pandruvada --- arch/x86/include/asm/topology.h | 2 + arch/x86/kernel/itmt.c | 105 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h index 1cd8d12..46ebdd1 100644 --- a/arch/x86/include/asm/topology.h +++ b/arch/x86/include/asm/topology.h @@ -155,6 +155,7 @@ extern bool x86_topology_update; #include DECLARE_PER_CPU_READ_MOSTLY(int, sched_core_priority); +extern unsigned int __read_mostly sysctl_sched_itmt_enabled; /* Interface to set priority of a cpu */ void sched_set_itmt_core_prio(int prio, int core_cpu); @@ -167,6 +168,7 @@ void sched_clear_itmt_support(void); #else /* CONFIG_SCHED_ITMT */ +#define sysctl_sched_itmt_enabled 0 static inline void sched_set_itmt_core_prio(int prio, int core_cpu) { } diff --git a/arch/x86/kernel/itmt.c b/arch/x86/kernel/itmt.c index 4be3d81..b104368 100644 --- a/arch/x86/kernel/itmt.c +++ b/arch/x86/kernel/itmt.c @@ -34,6 +34,67 @@ DEFINE_PER_CPU_READ_MOSTLY(int, sched_core_priority); /* Boolean to track if system has ITMT capabilities */ static bool __read_mostly sched_itmt_capable; +/* + * Boolean to control whether we want to move processes to cpu capable + * of higher turbo frequency for cpus supporting Intel Turbo Boost Max + * Technology 3.0. + * + * It can be set via /proc/sys/kernel/sched_itmt_enabled + */ +unsigned int __read_mostly sysctl_sched_itmt_enabled; + +static int sched_itmt_update_handler(struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, loff_t *ppos) +{ + int ret; + unsigned int old_sysctl; + + mutex_lock(&itmt_update_mutex); + + if (!sched_itmt_capable) { + mutex_unlock(&itmt_update_mutex); + return -EINVAL; + } + + old_sysctl = sysctl_sched_itmt_enabled; + ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); + + if (!ret && write && old_sysctl != sysctl_sched_itmt_enabled) { + x86_topology_update = true; + rebuild_sched_domains(); + } + + mutex_unlock(&itmt_update_mutex); + + return ret; +} + +static unsigned int zero; +static unsigned int one = 1; +static struct ctl_table itmt_kern_table[] = { + { + .procname = "sched_itmt_enabled", + .data = &sysctl_sched_itmt_enabled, + .maxlen = sizeof(unsigned int), + .mode = 0644, + .proc_handler = sched_itmt_update_handler, + .extra1 = &zero, + .extra2 = &one, + }, + {} +}; + +static struct ctl_table itmt_root_table[] = { + { + .procname = "kernel", + .mode = 0555, + .child = itmt_kern_table, + }, + {} +}; + +static struct ctl_table_header *itmt_sysctl_header; + /** * sched_set_itmt_support - Indicate platform supports ITMT * @@ -47,13 +108,40 @@ static bool __read_mostly sched_itmt_capable; * * This must be done only after sched_set_itmt_core_prio * has been called to set the cpus' priorities. + * + * It must not be called with cpu hot plug lock + * held as we need to acquire the lock to rebuild sched domains + * later. */ int sched_set_itmt_support(void) { mutex_lock(&itmt_update_mutex); + if (sched_itmt_capable) { + mutex_unlock(&itmt_update_mutex); + return 0; + } + + itmt_sysctl_header = register_sysctl_table(itmt_root_table); + if (!itmt_sysctl_header) { + mutex_unlock(&itmt_update_mutex); + return -ENOMEM; + } + sched_itmt_capable = true; + /* + * ITMT capability automatically enables ITMT + * scheduling for small systems (single node). + */ + if (topology_num_packages() == 1) + sysctl_sched_itmt_enabled = 1; + + if (sysctl_sched_itmt_enabled) { + x86_topology_update = true; + rebuild_sched_domains(); + } + mutex_unlock(&itmt_update_mutex); return 0; } @@ -64,13 +152,30 @@ int sched_set_itmt_support(void) * This function is used by the OS to indicate that it has * revoked the platform's support of ITMT feature. * + * It must not be called with cpu hot plug lock + * held as we need to acquire the lock to rebuild sched domains + * later. */ void sched_clear_itmt_support(void) { mutex_lock(&itmt_update_mutex); + if (!sched_itmt_capable) { + mutex_unlock(&itmt_update_mutex); + return; + } sched_itmt_capable = false; + if (itmt_sysctl_header) + unregister_sysctl_table(itmt_sysctl_header); + + if (sysctl_sched_itmt_enabled) { + /* disable sched_itmt if we are no longer ITMT capable */ + sysctl_sched_itmt_enabled = 0; + x86_topology_update = true; + rebuild_sched_domains(); + } + mutex_unlock(&itmt_update_mutex); }