From: Viresh Kumar <viresh.kumar@linaro.org> To: Dietmar Eggemann <dietmar.eggemann@arm.com> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Linux PM list <linux-pm@vger.kernel.org>, Russell King <linux@arm.linux.org.uk>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Russell King <rmk+kernel@armlinux.org.uk>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, Juri Lelli <juri.lelli@arm.com>, Vincent Guittot <vincent.guittot@linaor.org>, Peter Zijlstra <peterz@infradead.org>, Morten Rasmussen <morten.rasmussen@arm.com> Subject: Re: [PATCH 2/6] drivers base/arch_topology: frequency-invariant load-tracking support Date: Tue, 20 Jun 2017 11:47:18 +0530 [thread overview] Message-ID: <CAOh2x=k1Zr15vAq8LA0+Sg3Cz7o=dL5C-erkEx_py7g56b7BwA@mail.gmail.com> (raw) In-Reply-To: <20170608075513.12475-3-dietmar.eggemann@arm.com> On Thu, Jun 8, 2017 at 1:25 PM, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote: > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > static int > init_cpu_capacity_callback(struct notifier_block *nb, > @@ -185,6 +192,7 @@ init_cpu_capacity_callback(struct notifier_block *nb, > cpus_to_visit, > policy->related_cpus); > for_each_cpu(cpu, policy->related_cpus) { > + per_cpu(max_freq, cpu) = policy->cpuinfo.max_freq; I am not sure about this but why shouldn't we use policy->max here ? As that is the max, we can set the frequency to right now. > if (cap_parsing_failed) > continue; > raw_capacity[cpu] = topology_get_cpu_scale(NULL, cpu) * > @@ -195,8 +203,10 @@ init_cpu_capacity_callback(struct notifier_block *nb, > if (!cap_parsing_failed) { > topology_normalize_cpu_scale(); > kfree(raw_capacity); > + pr_debug("cpu_capacity: parsing done\n"); > + } else { > + pr_debug("cpu_capacity: max frequency parsing done\n"); > } > - pr_debug("cpu_capacity: parsing done\n"); > cap_parsing_done = true; > schedule_work(&parsing_done_work); > } > @@ -208,8 +218,38 @@ static struct notifier_block init_cpu_capacity_notifier = { > .notifier_call = init_cpu_capacity_callback, > }; > > +static void set_freq_scale(unsigned int cpu, unsigned long freq) > +{ > + unsigned long max = per_cpu(max_freq, cpu); > + > + if (!max) > + return; > + > + per_cpu(freq_scale, cpu) = (freq << SCHED_CAPACITY_SHIFT) / max; > +} > + > +static int set_freq_scale_callback(struct notifier_block *nb, > + unsigned long val, > + void *data) > +{ > + struct cpufreq_freqs *freq = data; > + > + switch (val) { > + case CPUFREQ_PRECHANGE: > + set_freq_scale(freq->cpu, freq->new); Any specific reason on why are we doing this from PRECHANGE and not POSTCHANGE ? i.e. we are doing this before the frequency is really updated. > + } > + > + return 0; > +} > + > +static struct notifier_block set_freq_scale_notifier = { > + .notifier_call = set_freq_scale_callback, > +}; > + > 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 ? -- viresh
WARNING: multiple messages have this Message-ID (diff)
From: viresh.kumar@linaro.org (Viresh Kumar) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/6] drivers base/arch_topology: frequency-invariant load-tracking support Date: Tue, 20 Jun 2017 11:47:18 +0530 [thread overview] Message-ID: <CAOh2x=k1Zr15vAq8LA0+Sg3Cz7o=dL5C-erkEx_py7g56b7BwA@mail.gmail.com> (raw) In-Reply-To: <20170608075513.12475-3-dietmar.eggemann@arm.com> On Thu, Jun 8, 2017 at 1:25 PM, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote: > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > static int > init_cpu_capacity_callback(struct notifier_block *nb, > @@ -185,6 +192,7 @@ init_cpu_capacity_callback(struct notifier_block *nb, > cpus_to_visit, > policy->related_cpus); > for_each_cpu(cpu, policy->related_cpus) { > + per_cpu(max_freq, cpu) = policy->cpuinfo.max_freq; I am not sure about this but why shouldn't we use policy->max here ? As that is the max, we can set the frequency to right now. > if (cap_parsing_failed) > continue; > raw_capacity[cpu] = topology_get_cpu_scale(NULL, cpu) * > @@ -195,8 +203,10 @@ init_cpu_capacity_callback(struct notifier_block *nb, > if (!cap_parsing_failed) { > topology_normalize_cpu_scale(); > kfree(raw_capacity); > + pr_debug("cpu_capacity: parsing done\n"); > + } else { > + pr_debug("cpu_capacity: max frequency parsing done\n"); > } > - pr_debug("cpu_capacity: parsing done\n"); > cap_parsing_done = true; > schedule_work(&parsing_done_work); > } > @@ -208,8 +218,38 @@ static struct notifier_block init_cpu_capacity_notifier = { > .notifier_call = init_cpu_capacity_callback, > }; > > +static void set_freq_scale(unsigned int cpu, unsigned long freq) > +{ > + unsigned long max = per_cpu(max_freq, cpu); > + > + if (!max) > + return; > + > + per_cpu(freq_scale, cpu) = (freq << SCHED_CAPACITY_SHIFT) / max; > +} > + > +static int set_freq_scale_callback(struct notifier_block *nb, > + unsigned long val, > + void *data) > +{ > + struct cpufreq_freqs *freq = data; > + > + switch (val) { > + case CPUFREQ_PRECHANGE: > + set_freq_scale(freq->cpu, freq->new); Any specific reason on why are we doing this from PRECHANGE and not POSTCHANGE ? i.e. we are doing this before the frequency is really updated. > + } > + > + return 0; > +} > + > +static struct notifier_block set_freq_scale_notifier = { > + .notifier_call = set_freq_scale_callback, > +}; > + > 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 ? -- viresh
next prev parent reply other threads:[~2017-06-20 6:17 UTC|newest] Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-06-08 7:55 [PATCH 0/6] arm, arm64: frequency- and cpu-invariant accounting support for task scheduler Dietmar Eggemann 2017-06-08 7:55 ` Dietmar Eggemann 2017-06-08 7:55 ` [PATCH 1/6] drivers base/arch_topology: prepare cpufreq policy notifier for frequency-invariant load-tracking support Dietmar Eggemann 2017-06-08 7:55 ` Dietmar Eggemann 2017-06-12 14:45 ` Vincent Guittot 2017-06-12 14:45 ` Vincent Guittot 2017-06-08 7:55 ` [PATCH 2/6] drivers base/arch_topology: " Dietmar Eggemann 2017-06-08 7:55 ` Dietmar Eggemann 2017-06-08 7:55 ` Dietmar Eggemann 2017-06-12 14:27 ` Vincent Guittot 2017-06-12 14:27 ` Vincent Guittot 2017-06-14 7:55 ` Dietmar Eggemann 2017-06-14 7:55 ` Dietmar Eggemann 2017-06-14 7:55 ` Dietmar Eggemann 2017-06-14 13:08 ` Vincent Guittot 2017-06-14 13:08 ` Vincent Guittot 2017-06-15 8:28 ` Juri Lelli 2017-06-15 8:28 ` Juri Lelli 2017-06-21 16:40 ` Dietmar Eggemann 2017-06-21 16:40 ` Dietmar Eggemann 2017-06-20 6:17 ` Viresh Kumar [this message] 2017-06-20 6:17 ` Viresh Kumar 2017-06-20 6:17 ` Viresh Kumar 2017-06-21 0:31 ` Saravana Kannan 2017-06-21 0:31 ` Saravana Kannan 2017-06-21 0:31 ` Saravana Kannan 2017-06-21 5:37 ` Viresh Kumar 2017-06-21 5:37 ` Viresh Kumar 2017-06-21 5:37 ` Viresh Kumar 2017-06-21 16:57 ` Morten Rasmussen 2017-06-21 16:57 ` Morten Rasmussen 2017-06-21 16:57 ` Morten Rasmussen 2017-06-22 4:06 ` Viresh Kumar 2017-06-22 4:06 ` Viresh Kumar 2017-06-22 4:06 ` Viresh Kumar 2017-06-22 9:59 ` Morten Rasmussen 2017-06-22 9:59 ` Morten Rasmussen 2017-06-22 9:59 ` Morten Rasmussen 2017-06-21 17:08 ` Dietmar Eggemann 2017-06-21 17:08 ` Dietmar Eggemann 2017-06-21 17:08 ` Dietmar Eggemann 2017-06-21 16:38 ` Dietmar Eggemann 2017-06-21 16:38 ` Dietmar Eggemann 2017-06-21 16:38 ` Dietmar Eggemann 2017-06-22 3:55 ` Viresh Kumar 2017-06-22 3:55 ` Viresh Kumar 2017-06-22 3:55 ` Viresh Kumar 2017-06-26 8:28 ` Dietmar Eggemann 2017-06-26 8:28 ` Dietmar Eggemann 2017-06-08 7:55 ` [PATCH 3/6] arm: wire frequency-invariant accounting support up to the task scheduler Dietmar Eggemann 2017-06-08 7:55 ` Dietmar Eggemann 2017-06-12 14:30 ` Vincent Guittot 2017-06-12 14:30 ` Vincent Guittot 2017-06-08 7:55 ` [PATCH 4/6] arm: wire cpu-invariant " Dietmar Eggemann 2017-06-08 7:55 ` Dietmar Eggemann 2017-06-12 14:31 ` Vincent Guittot 2017-06-12 14:31 ` Vincent Guittot 2017-06-08 7:55 ` [PATCH 5/6] arm64: wire frequency-invariant " Dietmar Eggemann 2017-06-08 7:55 ` Dietmar Eggemann 2017-06-12 13:06 ` Catalin Marinas 2017-06-12 13:06 ` Catalin Marinas 2017-06-12 14:32 ` Vincent Guittot 2017-06-12 14:32 ` Vincent Guittot 2017-06-08 7:55 ` [PATCH 6/6] arm64: wire cpu-invariant " Dietmar Eggemann 2017-06-08 7:55 ` Dietmar Eggemann 2017-06-12 13:07 ` Catalin Marinas 2017-06-12 13:07 ` Catalin Marinas 2017-06-12 14:33 ` Vincent Guittot 2017-06-12 14:33 ` Vincent Guittot 2017-06-12 13:00 ` [PATCH 0/6] arm, arm64: frequency- and cpu-invariant accounting support for " Juri Lelli 2017-06-12 13:00 ` Juri Lelli 2017-06-12 13:04 ` Juri Lelli 2017-06-12 13:04 ` Juri Lelli
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='CAOh2x=k1Zr15vAq8LA0+Sg3Cz7o=dL5C-erkEx_py7g56b7BwA@mail.gmail.com' \ --to=viresh.kumar@linaro.org \ --cc=catalin.marinas@arm.com \ --cc=dietmar.eggemann@arm.com \ --cc=gregkh@linuxfoundation.org \ --cc=juri.lelli@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=morten.rasmussen@arm.com \ --cc=peterz@infradead.org \ --cc=rmk+kernel@armlinux.org.uk \ --cc=vincent.guittot@linaor.org \ --cc=will.deacon@arm.com \ /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: linkBe 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.