From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753252AbdFURIa (ORCPT ); Wed, 21 Jun 2017 13:08:30 -0400 Received: from foss.arm.com ([217.140.101.70]:55852 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751126AbdFURI2 (ORCPT ); Wed, 21 Jun 2017 13:08:28 -0400 Subject: Re: [PATCH 2/6] drivers base/arch_topology: frequency-invariant load-tracking support To: Saravana Kannan , Viresh Kumar Cc: "linux-kernel@vger.kernel.org" , Linux PM list , Russell King , "linux-arm-kernel@lists.infradead.org" , Greg Kroah-Hartman , Russell King , Catalin Marinas , Will Deacon , Juri Lelli , Vincent Guittot , Peter Zijlstra , Morten Rasmussen References: <20170608075513.12475-1-dietmar.eggemann@arm.com> <20170608075513.12475-3-dietmar.eggemann@arm.com> <5949BE5F.4020502@codeaurora.org> From: Dietmar Eggemann Message-ID: <07ba559a-a270-db37-d43f-1540555a6369@arm.com> Date: Wed, 21 Jun 2017 18:08:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <5949BE5F.4020502@codeaurora.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 21/06/17 01:31, Saravana Kannan wrote: > On 06/19/2017 11:17 PM, Viresh Kumar wrote: >> On Thu, Jun 8, 2017 at 1:25 PM, Dietmar Eggemann >> wrote: >> >>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c >> >>> static int __init register_cpufreq_notifier(void) >>> { >>> + int ret; >>> + >>> /* >>> * on ACPI-based systems we need to use the default cpu >>> capacity >>> * until we have the necessary code to parse the cpu >>> capacity, so >>> @@ -225,8 +265,14 @@ static int __init register_cpufreq_notifier(void) >>> >>> cpumask_copy(cpus_to_visit, cpu_possible_mask); >>> >>> - return cpufreq_register_notifier(&init_cpu_capacity_notifier, >>> - CPUFREQ_POLICY_NOTIFIER); >>> + ret = cpufreq_register_notifier(&init_cpu_capacity_notifier, >>> + CPUFREQ_POLICY_NOTIFIER); >> >> Wanted to make sure that we all understand the constraints this is >> going to add >> for the ARM64 platforms. >> >> With the introduction of this transition notifier, we would not be >> able to use >> the fast-switch path in the schedutil governor. I am not sure if there >> are any >> ARM platforms that can actually use the fast-switch path in future or not >> though. The requirement of fast-switch path is that the freq can be >> changed >> without sleeping in the hot-path. >> >> So, will we ever want fast-switching for ARM platforms ? >> > > I don't think we should go down a path that'll prevent ARM platform from > switching over to fast-switching in the future. Understood. But IMHO implementing a cpufreq transition notifier based Frequency Invariance Engine (FIE) which provides frequency-invariant accounting for 100% of today's arm/arm64 system is legitimate. Like I said in the other email in this thread today, I can make sure that I only register the cpufreq transition notifier if none of the policies support fast frequency switching. In this case people can use mainline to experiment with cpufreq drivers supporting fast frequency switching (without FIE support). > Having said that, I'm not sure I fully agree with the decision to > completely disable notifiers in the fast-switching case. How many of the > current users of notifiers truly need support for sleeping in the > notifier? Why not make all the transition notifiers atomic? Or at least > add atomic transition notifiers that can be registered for separately if > the client doesn't need the ability to sleep? IMHO, that's a different construction side inside the cpufreq framework. Patches which introduced the fast frequency switching support in cpufreq clearly state that "... fast frequency switching is inherently incompatible with cpufreq transition notifiers ...". If we can get rid of this restriction, the cpufreq transition notifier based FIE implementation could stay. Otherwise we need a FIE for systems with fast frequency switching based on something else, e.g. performance counters. > Most of the clients don't seem like ones that'll need to sleep. > > There are a bunch of generic off-tree drivers (can't upstream them yet > because it depends on the bus scaling framework) that also depend on > CPUfreq transition notifiers that are going to stop working if fast > switching becomes available in the future. So, this decision to disallow > transition notifiers is painful for other reasons too. Falls into the same bucket for me ... you have a requirement against the cpufreq framework to let cpufreq transition notifier work for fast frequency switching drivers. [...] From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dietmar Eggemann Subject: Re: [PATCH 2/6] drivers base/arch_topology: frequency-invariant load-tracking support Date: Wed, 21 Jun 2017 18:08:25 +0100 Message-ID: <07ba559a-a270-db37-d43f-1540555a6369@arm.com> References: <20170608075513.12475-1-dietmar.eggemann@arm.com> <20170608075513.12475-3-dietmar.eggemann@arm.com> <5949BE5F.4020502@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: Received: from foss.arm.com ([217.140.101.70]:55852 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751126AbdFURI2 (ORCPT ); Wed, 21 Jun 2017 13:08:28 -0400 In-Reply-To: <5949BE5F.4020502@codeaurora.org> Content-Language: en-GB Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Saravana Kannan , Viresh Kumar Cc: "linux-kernel@vger.kernel.org" , Linux PM list , Russell King , "linux-arm-kernel@lists.infradead.org" , Greg Kroah-Hartman , Russell King , Catalin Marinas , Will Deacon , Juri Lelli , Vincent Guittot , Peter Zijlstra , Morten Rasmussen On 21/06/17 01:31, Saravana Kannan wrote: > On 06/19/2017 11:17 PM, Viresh Kumar wrote: >> On Thu, Jun 8, 2017 at 1:25 PM, Dietmar Eggemann >> wrote: >> >>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c >> >>> static int __init register_cpufreq_notifier(void) >>> { >>> + int ret; >>> + >>> /* >>> * on ACPI-based systems we need to use the default cpu >>> capacity >>> * until we have the necessary code to parse the cpu >>> capacity, so >>> @@ -225,8 +265,14 @@ static int __init register_cpufreq_notifier(void) >>> >>> cpumask_copy(cpus_to_visit, cpu_possible_mask); >>> >>> - return cpufreq_register_notifier(&init_cpu_capacity_notifier, >>> - CPUFREQ_POLICY_NOTIFIER); >>> + ret = cpufreq_register_notifier(&init_cpu_capacity_notifier, >>> + CPUFREQ_POLICY_NOTIFIER); >> >> Wanted to make sure that we all understand the constraints this is >> going to add >> for the ARM64 platforms. >> >> With the introduction of this transition notifier, we would not be >> able to use >> the fast-switch path in the schedutil governor. I am not sure if there >> are any >> ARM platforms that can actually use the fast-switch path in future or not >> though. The requirement of fast-switch path is that the freq can be >> changed >> without sleeping in the hot-path. >> >> So, will we ever want fast-switching for ARM platforms ? >> > > I don't think we should go down a path that'll prevent ARM platform from > switching over to fast-switching in the future. Understood. But IMHO implementing a cpufreq transition notifier based Frequency Invariance Engine (FIE) which provides frequency-invariant accounting for 100% of today's arm/arm64 system is legitimate. Like I said in the other email in this thread today, I can make sure that I only register the cpufreq transition notifier if none of the policies support fast frequency switching. In this case people can use mainline to experiment with cpufreq drivers supporting fast frequency switching (without FIE support). > Having said that, I'm not sure I fully agree with the decision to > completely disable notifiers in the fast-switching case. How many of the > current users of notifiers truly need support for sleeping in the > notifier? Why not make all the transition notifiers atomic? Or at least > add atomic transition notifiers that can be registered for separately if > the client doesn't need the ability to sleep? IMHO, that's a different construction side inside the cpufreq framework. Patches which introduced the fast frequency switching support in cpufreq clearly state that "... fast frequency switching is inherently incompatible with cpufreq transition notifiers ...". If we can get rid of this restriction, the cpufreq transition notifier based FIE implementation could stay. Otherwise we need a FIE for systems with fast frequency switching based on something else, e.g. performance counters. > Most of the clients don't seem like ones that'll need to sleep. > > There are a bunch of generic off-tree drivers (can't upstream them yet > because it depends on the bus scaling framework) that also depend on > CPUfreq transition notifiers that are going to stop working if fast > switching becomes available in the future. So, this decision to disallow > transition notifiers is painful for other reasons too. Falls into the same bucket for me ... you have a requirement against the cpufreq framework to let cpufreq transition notifier work for fast frequency switching drivers. [...] From mboxrd@z Thu Jan 1 00:00:00 1970 From: dietmar.eggemann@arm.com (Dietmar Eggemann) Date: Wed, 21 Jun 2017 18:08:25 +0100 Subject: [PATCH 2/6] drivers base/arch_topology: frequency-invariant load-tracking support In-Reply-To: <5949BE5F.4020502@codeaurora.org> References: <20170608075513.12475-1-dietmar.eggemann@arm.com> <20170608075513.12475-3-dietmar.eggemann@arm.com> <5949BE5F.4020502@codeaurora.org> Message-ID: <07ba559a-a270-db37-d43f-1540555a6369@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 21/06/17 01:31, Saravana Kannan wrote: > On 06/19/2017 11:17 PM, Viresh Kumar wrote: >> On Thu, Jun 8, 2017 at 1:25 PM, Dietmar Eggemann >> wrote: >> >>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c >> >>> static int __init register_cpufreq_notifier(void) >>> { >>> + int ret; >>> + >>> /* >>> * on ACPI-based systems we need to use the default cpu >>> capacity >>> * until we have the necessary code to parse the cpu >>> capacity, so >>> @@ -225,8 +265,14 @@ static int __init register_cpufreq_notifier(void) >>> >>> cpumask_copy(cpus_to_visit, cpu_possible_mask); >>> >>> - return cpufreq_register_notifier(&init_cpu_capacity_notifier, >>> - CPUFREQ_POLICY_NOTIFIER); >>> + ret = cpufreq_register_notifier(&init_cpu_capacity_notifier, >>> + CPUFREQ_POLICY_NOTIFIER); >> >> Wanted to make sure that we all understand the constraints this is >> going to add >> for the ARM64 platforms. >> >> With the introduction of this transition notifier, we would not be >> able to use >> the fast-switch path in the schedutil governor. I am not sure if there >> are any >> ARM platforms that can actually use the fast-switch path in future or not >> though. The requirement of fast-switch path is that the freq can be >> changed >> without sleeping in the hot-path. >> >> So, will we ever want fast-switching for ARM platforms ? >> > > I don't think we should go down a path that'll prevent ARM platform from > switching over to fast-switching in the future. Understood. But IMHO implementing a cpufreq transition notifier based Frequency Invariance Engine (FIE) which provides frequency-invariant accounting for 100% of today's arm/arm64 system is legitimate. Like I said in the other email in this thread today, I can make sure that I only register the cpufreq transition notifier if none of the policies support fast frequency switching. In this case people can use mainline to experiment with cpufreq drivers supporting fast frequency switching (without FIE support). > Having said that, I'm not sure I fully agree with the decision to > completely disable notifiers in the fast-switching case. How many of the > current users of notifiers truly need support for sleeping in the > notifier? Why not make all the transition notifiers atomic? Or at least > add atomic transition notifiers that can be registered for separately if > the client doesn't need the ability to sleep? IMHO, that's a different construction side inside the cpufreq framework. Patches which introduced the fast frequency switching support in cpufreq clearly state that "... fast frequency switching is inherently incompatible with cpufreq transition notifiers ...". If we can get rid of this restriction, the cpufreq transition notifier based FIE implementation could stay. Otherwise we need a FIE for systems with fast frequency switching based on something else, e.g. performance counters. > Most of the clients don't seem like ones that'll need to sleep. > > There are a bunch of generic off-tree drivers (can't upstream them yet > because it depends on the bus scaling framework) that also depend on > CPUfreq transition notifiers that are going to stop working if fast > switching becomes available in the future. So, this decision to disallow > transition notifiers is painful for other reasons too. Falls into the same bucket for me ... you have a requirement against the cpufreq framework to let cpufreq transition notifier work for fast frequency switching drivers. [...]