* [PATCH V7 5/7] cpufreq: Register notifiers with the PM QoS framework
From: Viresh Kumar @ 2019-07-08 10:57 UTC (permalink / raw)
To: Rafael Wysocki
Cc: Viresh Kumar, linux-pm, Vincent Guittot, mka, ulf.hansson, sfr,
pavel, Rafael J . Wysocki, linux-kernel
In-Reply-To: <5ad2624194baa2f53acc1f1e627eb7684c577a19.1562210705.git.viresh.kumar@linaro.org>
This registers the notifiers for min/max frequency constraints with the
PM QoS framework. The constraints are also taken into consideration in
cpufreq_set_policy().
This also relocates cpufreq_policy_put_kobj() as it is required to be
called from cpufreq_policy_alloc() now.
refresh_frequency_limits() is updated to avoid calling
cpufreq_set_policy() for inactive policies and handle_update() is
updated to have proper locking in place.
No constraints are added until now though.
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
V6->V7:
- All callers of refresh_frequency_limits(), except handle_update(),
take the policy->rwsem and result in deadlock as
refresh_frequency_limits() also takes the same lock again. Fix that
by taking the rwsem from handle_update() instead.
@Rafael: Sending it before Pavel has verified it as I would be offline
later, in case you want to apply this today itself.
drivers/cpufreq/cpufreq.c | 135 +++++++++++++++++++++++++++++---------
include/linux/cpufreq.h | 3 +
2 files changed, 108 insertions(+), 30 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index ceb57af15ca0..b96ef6db1bfe 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -26,6 +26,7 @@
#include <linux/kernel_stat.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/suspend.h>
#include <linux/syscore_ops.h>
@@ -999,7 +1000,7 @@ static void add_cpu_dev_symlink(struct cpufreq_policy *policy, unsigned int cpu)
{
struct device *dev = get_cpu_device(cpu);
- if (!dev)
+ if (unlikely(!dev))
return;
if (cpumask_test_and_set_cpu(cpu, policy->real_cpus))
@@ -1117,14 +1118,16 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cp
static void refresh_frequency_limits(struct cpufreq_policy *policy)
{
- struct cpufreq_policy new_policy = *policy;
-
- pr_debug("updating policy for CPU %u\n", policy->cpu);
+ struct cpufreq_policy new_policy;
- new_policy.min = policy->user_policy.min;
- new_policy.max = policy->user_policy.max;
+ if (!policy_is_inactive(policy)) {
+ new_policy = *policy;
+ pr_debug("updating policy for CPU %u\n", policy->cpu);
- cpufreq_set_policy(policy, &new_policy);
+ new_policy.min = policy->user_policy.min;
+ new_policy.max = policy->user_policy.max;
+ cpufreq_set_policy(policy, &new_policy);
+ }
}
static void handle_update(struct work_struct *work)
@@ -1133,14 +1136,60 @@ static void handle_update(struct work_struct *work)
container_of(work, struct cpufreq_policy, update);
pr_debug("handle_update for cpu %u called\n", policy->cpu);
+ down_write(&policy->rwsem);
refresh_frequency_limits(policy);
+ up_write(&policy->rwsem);
+}
+
+static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
+ void *data)
+{
+ struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_min);
+
+ schedule_work(&policy->update);
+ return 0;
+}
+
+static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
+ void *data)
+{
+ struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_max);
+
+ schedule_work(&policy->update);
+ return 0;
+}
+
+static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy)
+{
+ struct kobject *kobj;
+ struct completion *cmp;
+
+ down_write(&policy->rwsem);
+ cpufreq_stats_free_table(policy);
+ kobj = &policy->kobj;
+ cmp = &policy->kobj_unregister;
+ up_write(&policy->rwsem);
+ kobject_put(kobj);
+
+ /*
+ * We need to make sure that the underlying kobj is
+ * actually not referenced anymore by anybody before we
+ * proceed with unloading.
+ */
+ pr_debug("waiting for dropping of refcount\n");
+ wait_for_completion(cmp);
+ pr_debug("wait complete\n");
}
static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
{
struct cpufreq_policy *policy;
+ struct device *dev = get_cpu_device(cpu);
int ret;
+ if (!dev)
+ return NULL;
+
policy = kzalloc(sizeof(*policy), GFP_KERNEL);
if (!policy)
return NULL;
@@ -1157,7 +1206,7 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
ret = kobject_init_and_add(&policy->kobj, &ktype_cpufreq,
cpufreq_global_kobject, "policy%u", cpu);
if (ret) {
- pr_err("%s: failed to init policy->kobj: %d\n", __func__, ret);
+ dev_err(dev, "%s: failed to init policy->kobj: %d\n", __func__, ret);
/*
* The entire policy object will be freed below, but the extra
* memory allocated for the kobject name needs to be freed by
@@ -1167,6 +1216,25 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
goto err_free_real_cpus;
}
+ policy->nb_min.notifier_call = cpufreq_notifier_min;
+ policy->nb_max.notifier_call = cpufreq_notifier_max;
+
+ ret = dev_pm_qos_add_notifier(dev, &policy->nb_min,
+ DEV_PM_QOS_MIN_FREQUENCY);
+ if (ret) {
+ dev_err(dev, "Failed to register MIN QoS notifier: %d (%*pbl)\n",
+ ret, cpumask_pr_args(policy->cpus));
+ goto err_kobj_remove;
+ }
+
+ ret = dev_pm_qos_add_notifier(dev, &policy->nb_max,
+ DEV_PM_QOS_MAX_FREQUENCY);
+ if (ret) {
+ dev_err(dev, "Failed to register MAX QoS notifier: %d (%*pbl)\n",
+ ret, cpumask_pr_args(policy->cpus));
+ goto err_min_qos_notifier;
+ }
+
INIT_LIST_HEAD(&policy->policy_list);
init_rwsem(&policy->rwsem);
spin_lock_init(&policy->transition_lock);
@@ -1177,6 +1245,11 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
policy->cpu = cpu;
return policy;
+err_min_qos_notifier:
+ dev_pm_qos_remove_notifier(dev, &policy->nb_min,
+ DEV_PM_QOS_MIN_FREQUENCY);
+err_kobj_remove:
+ cpufreq_policy_put_kobj(policy);
err_free_real_cpus:
free_cpumask_var(policy->real_cpus);
err_free_rcpumask:
@@ -1189,30 +1262,9 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
return NULL;
}
-static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy)
-{
- struct kobject *kobj;
- struct completion *cmp;
-
- down_write(&policy->rwsem);
- cpufreq_stats_free_table(policy);
- kobj = &policy->kobj;
- cmp = &policy->kobj_unregister;
- up_write(&policy->rwsem);
- kobject_put(kobj);
-
- /*
- * We need to make sure that the underlying kobj is
- * actually not referenced anymore by anybody before we
- * proceed with unloading.
- */
- pr_debug("waiting for dropping of refcount\n");
- wait_for_completion(cmp);
- pr_debug("wait complete\n");
-}
-
static void cpufreq_policy_free(struct cpufreq_policy *policy)
{
+ struct device *dev = get_cpu_device(policy->cpu);
unsigned long flags;
int cpu;
@@ -1224,6 +1276,11 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
per_cpu(cpufreq_cpu_data, cpu) = NULL;
write_unlock_irqrestore(&cpufreq_driver_lock, flags);
+ dev_pm_qos_remove_notifier(dev, &policy->nb_max,
+ DEV_PM_QOS_MAX_FREQUENCY);
+ dev_pm_qos_remove_notifier(dev, &policy->nb_min,
+ DEV_PM_QOS_MIN_FREQUENCY);
+
cpufreq_policy_put_kobj(policy);
free_cpumask_var(policy->real_cpus);
free_cpumask_var(policy->related_cpus);
@@ -2283,6 +2340,8 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
struct cpufreq_policy *new_policy)
{
struct cpufreq_governor *old_gov;
+ struct device *cpu_dev = get_cpu_device(policy->cpu);
+ unsigned long min, max;
int ret;
pr_debug("setting new policy for CPU %u: %u - %u kHz\n",
@@ -2297,11 +2356,27 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
if (new_policy->min > new_policy->max)
return -EINVAL;
+ /*
+ * PM QoS framework collects all the requests from users and provide us
+ * the final aggregated value here.
+ */
+ min = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MIN_FREQUENCY);
+ max = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MAX_FREQUENCY);
+
+ if (min > new_policy->min)
+ new_policy->min = min;
+ if (max < new_policy->max)
+ new_policy->max = max;
+
/* verify the cpu speed can be set within this limit */
ret = cpufreq_driver->verify(new_policy);
if (ret)
return ret;
+ /*
+ * The notifier-chain shall be removed once all the users of
+ * CPUFREQ_ADJUST are moved to use the QoS framework.
+ */
/* adjust if necessary - all reasons */
blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
CPUFREQ_ADJUST, new_policy);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index a1467aa7f58b..95425941f46d 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -147,6 +147,9 @@ struct cpufreq_policy {
/* Pointer to the cooling device if used for thermal mitigation */
struct thermal_cooling_device *cdev;
+
+ struct notifier_block nb_min;
+ struct notifier_block nb_max;
};
struct cpufreq_freqs {
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related
* Re: cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60
From: Rafael J. Wysocki @ 2019-07-08 10:47 UTC (permalink / raw)
To: Viresh Kumar
Cc: Pavel Machek, Rafael J. Wysocki, Matthias Kaehlcke, Ulf Hansson,
Rafael J. Wysocki, Linux-pm mailing list, kernel list,
Stephen Rothwell
In-Reply-To: <20190708092840.ynibtrntval6krc4@vireshk-i7>
On Mon, Jul 8, 2019 at 11:28 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 08-07-19, 10:28, Rafael J. Wysocki wrote:
> > Pavel has tested the latest version of the patch series AFAICS.
> >
> > The locking added by the commit in question to
> > refresh_frequency_limits() requires an update of
> > cpufreq_update_policy(), or it will deadlock in there because of the
> > lock acquired by cpufreq_cpu_get() if I haven't missed anything.
>
> Ah, looks quite straight forward.
>
> @Pavel: Can you please try this diff ?
>
> -------------------------8<-------------------------
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 9f68d0f306b8..4d6043ee7834 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1109,16 +1109,12 @@ void refresh_frequency_limits(struct cpufreq_policy *policy)
> {
> struct cpufreq_policy new_policy;
>
> - down_write(&policy->rwsem);
> -
> if (!policy_is_inactive(policy)) {
> new_policy = *policy;
> pr_debug("updating policy for CPU %u\n", policy->cpu);
>
> cpufreq_set_policy(policy, &new_policy);
> }
> -
> - up_write(&policy->rwsem);
> }
> EXPORT_SYMBOL(refresh_frequency_limits);
>
> @@ -1128,7 +1124,9 @@ static void handle_update(struct work_struct *work)
> container_of(work, struct cpufreq_policy, update);
>
> pr_debug("handle_update for cpu %u called\n", policy->cpu);
> + down_write(&policy->rwsem);
> refresh_frequency_limits(policy);
> + up_write(&policy->rwsem);
> }
>
> -------------------------8<-------------------------
>
> Though it makes me wonder why I didn't hit this thing. I was using the
> cpu_cooling device the other day, which calls cpufreq_update_policy()
> very frequently on heat-up. And I had a hair dryer blowing over my
> board to heat it up. Lemme check that again :)
>
> @Rafael: You want me to send a new diff patch with Fixes tag this time
> if this works out fine ?
I would prefer the original patch to be updated to avoid possible
bisection woes in the future.
^ permalink raw reply
* Re: [PATCH] cpufreq: schedutil: Fix covert rate_limit_us to freq_update_delay_ns overflow
From: zhangxiaoxu (A) @ 2019-07-08 10:47 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, mingo, peterz, linux-pm
In-Reply-To: <20190708093740.pbcnl7ytjmmpy3bz@vireshk-i7>
在 2019/7/8 17:37, Viresh Kumar 写道:
> On 08-07-19, 16:46, ZhangXiaoxu wrote:
>> When covert rate_limit_us to freq_update_delay_ns, it maybe overflow
>> and lead an undefined behavior.
>
> freq_update_delay_ns is s64, still overflow can happen ?
It will not happened. sorry for my misunderstander :(
>
>> So, limit the rate_limit_us to UINT_MAX / NSEC_PER_USEC.
>>
>> Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
>> ---
>> kernel/sched/cpufreq_schedutil.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
>> index 962cf34..01e05f3 100644
>> --- a/kernel/sched/cpufreq_schedutil.c
>> +++ b/kernel/sched/cpufreq_schedutil.c
>> @@ -590,6 +590,9 @@ rate_limit_us_store(struct gov_attr_set *attr_set, const char *buf, size_t count
>> if (kstrtouint(buf, 10, &rate_limit_us))
>> return -EINVAL;
>>
>> + if (rate_limit_us > UINT_MAX / NSEC_PER_USEC)
>> + return -EINVAL;
>> +
>> tunables->rate_limit_us = rate_limit_us;
>>
>> list_for_each_entry(sg_policy, &attr_set->policy_list, tunables_hook)
>> --
>> 2.7.4
>
^ permalink raw reply
* Re: [PATCH] cpuidle/drivers/mobile: Add new governor for mobile/embedded systems
From: Daniel Lezcano @ 2019-07-08 9:57 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: rafael, linux-kernel, Thomas Gleixner, Greg Kroah-Hartman,
open list:CPU IDLE TIME MANAGEMENT FRAMEWORK
In-Reply-To: <8334994.XHaO1tGd4f@kreacher>
Hi Rafael,
On 04/07/2019 12:14, Rafael J. Wysocki wrote:
> On Thursday, June 20, 2019 1:58:08 PM CEST Daniel Lezcano wrote:
>> The objective is the same for all the governors: save energy, but at
>> the end the governors menu, ladder and teo aim to improve the
>> performances with an acceptable energy drop for some workloads which
>> are identified for servers and desktops (with the help of a firmware).
>>
>> The ladder governor is designed for server with a periodic tick
>> configuration.
>>
>> The menu governor does not behave nicely with the mobile platform and
>> the energy saving for the multimedia workloads is worst than picking
>> up randomly an idle state.
>>
>> The teo governor acts efficiently, it promotes shallower state for
>> performances which is perfect for the servers / desktop but inadequate
>> for mobile because the energy consumed is too high.
>>
>> It is very difficult to do changes in these governors for embedded
>> systems without impacting performances on servers/desktops or ruin the
>> optimizations for the workloads on these platforms.
>>
>> The mobile governor is a new governor targeting embedded systems
>> running on battery where the energy saving has a higher priority than
>> servers or desktops. This governor aims to save energy as much as
>> possible but with a performance degradation tolerance.
>>
>> In this way, we can optimize the governor for specific mobile workload
>> and more generally embedded systems without impacting other platforms.
>>
>> The mobile governor is built on top of the paradigm 'separate the wake
>> up sources signals and analyze them'. Three categories of wake up
>> signals are identified:
>> - deterministic : timers
>> - predictable : most of the devices interrupt
>> - unpredictable : IPI rescheduling, random signals
>>
>> The latter needs an iterative approach and the help of the scheduler
>> to give more input to the governor.
>>
>> The governor uses the irq timings where we predict the next interrupt
>> occurrences on the current CPU and the next timer. It is well suited
>> for mobile and more generally embedded systems where the interrupts
>> are usually pinned on one CPU and where the power is more important
>> than the performances.
>>
>> The multimedia applications on the embedded system spawn multiple
>> threads which are migrated across the different CPUs and waking
>> between them up. In order to catch this situation we have also to
>> track the idle task rescheduling duration with a relative degree of
>> confidence as the scheduler is involved in the task migrations. The
>> resched information is in the scope of the governor via the reflect
>> callback.
>>
>> The governor begins with a clean foundation basing the prediction on
>> the irq behavior returned by the irq timings, the timers and the idle
>> task rescheduling. The advantage of the approach is we have a full
>> view of the wakeup sources as we identify them separately and then we
>> can control the situation without relying on biased heuristics.
>>
>> This first iteration provides a basic prediction but improves on some
>> mobile platforms better energy for better performance for multimedia
>> workloads.
>>
>> The scheduling aspect will be optimized iteratively with non
>> regression testing for previous identified workloads on an Android
>> reference platform.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> Note that there are build issues reported by 0-day that need to be fixed.
> Also, IMO this really should be documented better in the tree, not just in the changelog.
> At least the use case to be covered by this governor should be clearly documented and
> it would be good to describe the algorithm.
Ok, I will add some documentation.
>> ---
>> drivers/cpuidle/Kconfig | 11 ++-
>> drivers/cpuidle/governors/Makefile | 1 +
>> drivers/cpuidle/governors/mobile.c | 151 +++++++++++++++++++++++++++++
>> 3 files changed, 162 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/cpuidle/governors/mobile.c
>>
>> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
>> index a4ac31e4a58c..e2376d85e288 100644
>> --- a/drivers/cpuidle/Kconfig
>> +++ b/drivers/cpuidle/Kconfig
>> @@ -5,7 +5,7 @@ config CPU_IDLE
>> bool "CPU idle PM support"
>> default y if ACPI || PPC_PSERIES
>> select CPU_IDLE_GOV_LADDER if (!NO_HZ && !NO_HZ_IDLE)
>> - select CPU_IDLE_GOV_MENU if (NO_HZ || NO_HZ_IDLE) && !CPU_IDLE_GOV_TEO
>> + select CPU_IDLE_GOV_MENU if (NO_HZ || NO_HZ_IDLE) && !CPU_IDLE_GOV_TEO && !CPU_IDLE_GOV_MOBILE
>> help
>> CPU idle is a generic framework for supporting software-controlled
>> idle processor power management. It includes modular cross-platform
>> @@ -33,6 +33,15 @@ config CPU_IDLE_GOV_TEO
>> Some workloads benefit from using it and it generally should be safe
>> to use. Say Y here if you are not happy with the alternatives.
>>
>> +config CPU_IDLE_GOV_MOBILE
>> + bool "Mobile governor"
>> + select IRQ_TIMINGS
>> + help
>> + The mobile governor is based on irq timings measurements and
>> + pattern research combined with the next timer. This governor
>> + suits very well on embedded systems where the interrupts are
>> + grouped on a single core and the power is the priority.
>> +
>> config DT_IDLE_STATES
>> bool
>>
>> diff --git a/drivers/cpuidle/governors/Makefile b/drivers/cpuidle/governors/Makefile
>> index 42f44cc610dd..f09da7178670 100644
>> --- a/drivers/cpuidle/governors/Makefile
>> +++ b/drivers/cpuidle/governors/Makefile
>> @@ -6,3 +6,4 @@
>> obj-$(CONFIG_CPU_IDLE_GOV_LADDER) += ladder.o
>> obj-$(CONFIG_CPU_IDLE_GOV_MENU) += menu.o
>> obj-$(CONFIG_CPU_IDLE_GOV_TEO) += teo.o
>> +obj-$(CONFIG_CPU_IDLE_GOV_MOBILE) += mobile.o
>> diff --git a/drivers/cpuidle/governors/mobile.c b/drivers/cpuidle/governors/mobile.c
>> new file mode 100644
>> index 000000000000..8fda0f9b960b
>> --- /dev/null
>> +++ b/drivers/cpuidle/governors/mobile.c
>> @@ -0,0 +1,151 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2019, Linaro Ltd
>> + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
>> + */
>> +#include <linux/cpuidle.h>
>> +#include <linux/kernel.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/tick.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/sched/clock.h>
>> +
>> +struct mobile_device {
>> + u64 idle_ema_avg;
>> + u64 idle_total;
>> + unsigned long last_jiffies;
>> +};
>> +
>> +#define EMA_ALPHA_VAL 64
>> +#define EMA_ALPHA_SHIFT 7
>> +#define MAX_RESCHED_INTERVAL_MS 100
>> +
>> +static DEFINE_PER_CPU(struct mobile_device, mobile_devices);
>> +
>> +static int mobile_ema_new(s64 value, s64 ema_old)
>> +{
>> + if (likely(ema_old))
>> + return ema_old + (((value - ema_old) * EMA_ALPHA_VAL) >>
>> + EMA_ALPHA_SHIFT);
>> + return value;
>> +}
>> +
>> +static void mobile_reflect(struct cpuidle_device *dev, int index)
>> +{
>> + struct mobile_device *mobile_dev = this_cpu_ptr(&mobile_devices);
>> + struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
>> + struct cpuidle_state *s = &drv->states[index];
>> + int residency;
>> +
>> + /*
>> + * The idle task was not rescheduled since
>> + * MAX_RESCHED_INTERVAL_MS, let's consider the duration is
>> + * long enough to clear our stats.
>> + */
>> + if (time_after(jiffies, mobile_dev->last_jiffies +
>> + msecs_to_jiffies(MAX_RESCHED_INTERVAL_MS)))
>> + mobile_dev->idle_ema_avg = 0;
>
> Why jiffies? Any particular reason?
I used jiffies to not use the local_clock() call for the overhead. I
agree the resolution could be too low. Perhaps it makes more sense to
move idle start and idle end variable from the cpuidle_enter function to
the cpuidle device structure, so the information can be reused by the
subsequent users.
>> +
>> + /*
>> + * Sum all the residencies in order to compute the total
>> + * duration of the idle task.
>> + */
>> + residency = dev->last_residency - s->exit_latency;
>> + if (residency > 0)
>> + mobile_dev->idle_total += residency;
>> +
>> + /*
>> + * We exited the idle state with the need_resched() flag, the
>> + * idle task will be rescheduled, so store the duration the
>> + * idle task was scheduled in an exponential moving average and
>> + * reset the total of the idle duration.
>> + */
>> + if (need_resched()) {
>> + mobile_dev->idle_ema_avg = mobile_ema_new(mobile_dev->idle_total,
>> + mobile_dev->idle_ema_avg);
>> + mobile_dev->idle_total = 0;
>> + mobile_dev->last_jiffies = jiffies;
>> + }
>> +}
>> +
>> +static int mobile_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
>> + bool *stop_tick)
>> +{
>> + struct mobile_device *mobile_dev = this_cpu_ptr(&mobile_devices);
>> + int latency_req = cpuidle_governor_latency_req(dev->cpu);
>> + int i, index = 0;
>> + ktime_t delta_next;
>> + u64 now, irq_length, timer_length;
>> + u64 idle_duration_us;
>> +
>> + /*
>> + * Get the present time as reference for the next steps
>> + */
>> + now = local_clock();
>> +
>> + /*
>> + * Get the next interrupt event giving the 'now' as a
>> + * reference, if the next event appears to have already
>> + * expired then we get the 'now' returned which ends up with a
>> + * zero duration.
>> + */
>> + irq_length = irq_timings_next_event(now) - now;
>> +
>> + /*
>> + * Get the timer duration before expiration.
>> + */
>
> This comment is rather redundant and the one below too. :-)
Right.
>> + timer_length = ktime_to_ns(tick_nohz_get_sleep_length(&delta_next));
>> +
>> + /*
>> + * Get the smallest duration between the timer and the irq next event.
>> + */
>> + idle_duration_us = min_t(u64, irq_length, timer_length) / NSEC_PER_USEC;
>> +
>> + /*
>> + * Get the idle task duration average if the information is
>> + * available.
>
> IMO it would be good to explain this step in more detail, especially the purpose of it.
Ok.
>> + */
>> + if (mobile_dev->idle_ema_avg)
>> + idle_duration_us = min_t(u64, idle_duration_us,
>> + mobile_dev->idle_ema_avg);
>> +
>> + for (i = 0; i < drv->state_count; i++) {
>> + struct cpuidle_state *s = &drv->states[i];
>> + struct cpuidle_state_usage *su = &dev->states_usage[i];
>> +
>> + if (s->disabled || su->disable)
>> + continue;
>> +
>> + if (s->exit_latency > latency_req)
>> + break;
>> +
>> + if (idle_duration_us > s->exit_latency)
>> + idle_duration_us = idle_duration_us - s->exit_latency;
>
> Why do you want this?
>
> It only causes you to miss an opportunity to select a deeper state sometimes,
> so what's the reason?
On the mobile platform the exit latencies are very high (with an order
of magnitude of several milliseconds) for a very limited number of idle
states. The real idle duration must be determined to compare to the
target residency. Without this test, the governor is constantly choosing
wrongly a deep idle state.
> Moreover, I don't think you should update idle_duration_us here, as the updated
> value will go to the next step if the check below doesn't trigger.
Right, I spotted it also and fixed with:
+ if (s->exit_latency >= idle_duration_us)
+ break;
+ if (s->target_residency > (idle_duration_us -
s->exit_latency))
break;
>> +
>> + if (s->target_residency > idle_duration_us)
>> + break;
>> +
>> + index = i;
>> + }
>> +
>> + if (!index)
>> + *stop_tick = false;
>
> Well, this means that the tick is stopped for all idle states deeper than state 0.
>
> If there are any states between state 0 and the deepest one and they are below
> the tick boundary, you may very well suffer the "powernightmares" problem
> because of this.
What would you suggest?
if (!index || ((idle_duration_us < TICK_USEC) &&
!tick_nohz_tick_stopped()))
*stop_tick = false;
?
There are too few idle states to restart a selection at this point, so
preventing stopping the tick is enough at this point IMO.
>> + return index;
>> +}
>> +
>> +static struct cpuidle_governor mobile_governor = {
>> + .name = "mobile",
>> + .rating = 20,
>> + .select = mobile_select,
>> + .reflect = mobile_reflect,
>> +};
>> +
>> +static int __init init_governor(void)
>> +{
>> + irq_timings_enable();
>> + return cpuidle_register_governor(&mobile_governor);
>> +}
>> +
>> +postcore_initcall(init_governor);
>>
>
>
>
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH] cpufreq: schedutil: Fix covert rate_limit_us to freq_update_delay_ns overflow
From: Viresh Kumar @ 2019-07-08 9:37 UTC (permalink / raw)
To: ZhangXiaoxu; +Cc: rjw, mingo, peterz, linux-pm
In-Reply-To: <1562575583-99575-1-git-send-email-zhangxiaoxu5@huawei.com>
On 08-07-19, 16:46, ZhangXiaoxu wrote:
> When covert rate_limit_us to freq_update_delay_ns, it maybe overflow
> and lead an undefined behavior.
freq_update_delay_ns is s64, still overflow can happen ?
> So, limit the rate_limit_us to UINT_MAX / NSEC_PER_USEC.
>
> Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
> ---
> kernel/sched/cpufreq_schedutil.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index 962cf34..01e05f3 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -590,6 +590,9 @@ rate_limit_us_store(struct gov_attr_set *attr_set, const char *buf, size_t count
> if (kstrtouint(buf, 10, &rate_limit_us))
> return -EINVAL;
>
> + if (rate_limit_us > UINT_MAX / NSEC_PER_USEC)
> + return -EINVAL;
> +
> tunables->rate_limit_us = rate_limit_us;
>
> list_for_each_entry(sg_policy, &attr_set->policy_list, tunables_hook)
> --
> 2.7.4
--
viresh
^ permalink raw reply
* Re: cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60
From: Viresh Kumar @ 2019-07-08 9:28 UTC (permalink / raw)
To: Pavel Machek, Rafael J. Wysocki
Cc: Matthias Kaehlcke, Ulf Hansson, Rafael J. Wysocki,
Linux-pm mailing list, kernel list, Stephen Rothwell
In-Reply-To: <CAJZ5v0hTXtjkatT4wVftPac-LgL1GSAbwLZ0mMDSpFn=8k9Ssg@mail.gmail.com>
On 08-07-19, 10:28, Rafael J. Wysocki wrote:
> Pavel has tested the latest version of the patch series AFAICS.
>
> The locking added by the commit in question to
> refresh_frequency_limits() requires an update of
> cpufreq_update_policy(), or it will deadlock in there because of the
> lock acquired by cpufreq_cpu_get() if I haven't missed anything.
Ah, looks quite straight forward.
@Pavel: Can you please try this diff ?
-------------------------8<-------------------------
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 9f68d0f306b8..4d6043ee7834 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1109,16 +1109,12 @@ void refresh_frequency_limits(struct cpufreq_policy *policy)
{
struct cpufreq_policy new_policy;
- down_write(&policy->rwsem);
-
if (!policy_is_inactive(policy)) {
new_policy = *policy;
pr_debug("updating policy for CPU %u\n", policy->cpu);
cpufreq_set_policy(policy, &new_policy);
}
-
- up_write(&policy->rwsem);
}
EXPORT_SYMBOL(refresh_frequency_limits);
@@ -1128,7 +1124,9 @@ static void handle_update(struct work_struct *work)
container_of(work, struct cpufreq_policy, update);
pr_debug("handle_update for cpu %u called\n", policy->cpu);
+ down_write(&policy->rwsem);
refresh_frequency_limits(policy);
+ up_write(&policy->rwsem);
}
-------------------------8<-------------------------
Though it makes me wonder why I didn't hit this thing. I was using the
cpu_cooling device the other day, which calls cpufreq_update_policy()
very frequently on heat-up. And I had a hair dryer blowing over my
board to heat it up. Lemme check that again :)
@Rafael: You want me to send a new diff patch with Fixes tag this time
if this works out fine ?
--
viresh
^ permalink raw reply related
* Re: [PATCH V2 00/13] intel_rapl: RAPL abstraction and MMIO RAPL support
From: Rafael J. Wysocki @ 2019-07-08 8:58 UTC (permalink / raw)
To: Zhang Rui
Cc: Rafael J. Wysocki, Pandruvada, Srinivas, linux-pm@vger.kernel.org,
rjw@rjwysocki.net
In-Reply-To: <1562568815.2810.18.camel@intel.com>
On Mon, Jul 8, 2019 at 8:53 AM Zhang Rui <rui.zhang@intel.com> wrote:
>
> On 六, 2019-07-06 at 10:19 +0200, Rafael J. Wysocki wrote:
> > On Fri, Jul 5, 2019 at 4:59 PM Pandruvada, Srinivas
> > <srinivas.pandruvada@intel.com> wrote:
> > >
> > >
> > > On Fri, 2019-07-05 at 10:57 +0200, Rafael J. Wysocki wrote:
> > > >
> > > > On Thu, Jul 4, 2019 at 6:34 PM Zhang Rui <rui.zhang@intel.com>
> > > > wrote:
> > > > >
> > > > >
> > > > > Besideis MSR interface, RAPL can also be controlled via the
> > > > > MMIO
> > > > > interface,
> > > > > by accessing the MCHBar registers exposed by the processor
> > > > > thermal
> > > > > device.
> > > > >
> > > > > Currently, we only have RAPL MSR interface in Linux kernel,
> > > > > this
> > > > > brings
> > > > > problems on some platforms that BIOS performs a low power
> > > > > limits
> > > > > via the
> > > > > MMIO interface by default. This results in poor system
> > > > > performance,
> > > > > and there is no way for us to change the MMIO MSR setting in
> > > > > Linux.
> > > > >
> > > > > To fix this, RAPL MMIO interface support is introduced in this
> > > > > patch set.
> > > > >
> > > > > Patch 1/13 to patch 11/13 abstract the RAPL code, and move all
> > > > > the
> > > > > shared
> > > > > code into a separate file, intel_rapl_common.c, so that it can
> > > > > be
> > > > > used
> > > > > by both MSR and MMIO interfaces.
> > > > > Patch 12/13 introduced RAPL support via MMIO registers, exposed
> > > > > by
> > > > > the
> > > > > processor thermal devices.
> > > > > Patch 13/13 fixes a module autoloading issue found later.
> > > > >
> > > > > The patch series has been tested on Dell XPS 9360, a SKL
> > > > > platform.
> > > > >
> > > > > Note that this patch series are based on the -tip tree, which
> > > > > contains the
> > > > > latest RAPL changes for multi-die support.
> > > > >
> > > > > Changes in V2:
> > > > > - add kerneldoc for struct rapl_if_priv.
> > > > > - use intel_rapl_msr.c for RAPL MSR I/F driver, instead of
> > > > > intel_rapl.c.
> > > > > - changelog and coding style update.
> > > > What tree is the series against?
> > > >
> > > > It doesn't apply either on top of my powercap branch or on top of
> > > > 5.2-rc7 for me.
> > > This needs linux tip tree. There are some package/die changes in
> > > tip
> > > tree, which this patch depends on.
> > OK, so the changes in -tip need to go in first.
> >
> exactly.
> BTW, this patch set also conflicts with the RAPL support patches for
> icelake platforms.
Do you mean commits
cc3ae777098b..88679b2587a0
in linux-next?
> Thus IMO, having a separate rapl branch, and apply
> the icl rapl patches on top of this patch set will be much easier.
OK, I'll do that if that's preferred.
^ permalink raw reply
* Re: [PATCH v3] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()
From: Viresh Kumar @ 2019-07-08 8:56 UTC (permalink / raw)
To: Wen Yang
Cc: linux-kernel, xue.zhihong, wang.yi59, cheng.shengyu,
Rafael J. Wysocki, linuxppc-dev, linux-pm
In-Reply-To: <1562575726-17438-1-git-send-email-wen.yang99@zte.com.cn>
On 08-07-19, 16:48, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
>
> Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> v3: fix a leaked reference.
> v2: clean up the code according to the advice of viresh.
>
> drivers/cpufreq/pasemi-cpufreq.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> index 6b1e4ab..9dc5163 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -128,20 +128,20 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> int cur_astate, idx;
> struct resource res;
> struct device_node *cpu, *dn;
> - int err = -ENODEV;
> + int err;
>
> cpu = of_get_cpu_node(policy->cpu, NULL);
> -
> - of_node_put(cpu);
> if (!cpu)
> - goto out;
> + return -ENODEV;
>
> dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
> if (!dn)
> dn = of_find_compatible_node(NULL, NULL,
> "pasemi,pwrficient-sdc");
> - if (!dn)
> + if (!dn) {
> + err = -ENODEV;
> goto out;
> + }
Please restore the blank line here.
> err = of_address_to_resource(dn, 0, &res);
> of_node_put(dn);
> if (err)
> @@ -196,6 +196,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> policy->cur = pas_freqs[cur_astate].frequency;
> ppc_proc_freq = policy->cur * 1000ul;
>
> + of_node_put(cpu);
> return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
>
> out_unmap_sdcpwr:
> @@ -204,6 +205,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> out_unmap_sdcasr:
> iounmap(sdcasr_mapbase);
> out:
> + of_node_put(cpu);
> return err;
> }
>
> --
> 2.9.5
--
viresh
^ permalink raw reply
* [PATCH v3] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()
From: Wen Yang @ 2019-07-08 8:48 UTC (permalink / raw)
To: linux-kernel
Cc: xue.zhihong, wang.yi59, cheng.shengyu, Wen Yang,
Rafael J. Wysocki, Viresh Kumar, linuxppc-dev, linux-pm
The cpu variable is still being used in the of_get_property() call
after the of_node_put() call, which may result in use-after-free.
Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
v3: fix a leaked reference.
v2: clean up the code according to the advice of viresh.
drivers/cpufreq/pasemi-cpufreq.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
index 6b1e4ab..9dc5163 100644
--- a/drivers/cpufreq/pasemi-cpufreq.c
+++ b/drivers/cpufreq/pasemi-cpufreq.c
@@ -128,20 +128,20 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
int cur_astate, idx;
struct resource res;
struct device_node *cpu, *dn;
- int err = -ENODEV;
+ int err;
cpu = of_get_cpu_node(policy->cpu, NULL);
-
- of_node_put(cpu);
if (!cpu)
- goto out;
+ return -ENODEV;
dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
if (!dn)
dn = of_find_compatible_node(NULL, NULL,
"pasemi,pwrficient-sdc");
- if (!dn)
+ if (!dn) {
+ err = -ENODEV;
goto out;
+ }
err = of_address_to_resource(dn, 0, &res);
of_node_put(dn);
if (err)
@@ -196,6 +196,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
policy->cur = pas_freqs[cur_astate].frequency;
ppc_proc_freq = policy->cur * 1000ul;
+ of_node_put(cpu);
return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
out_unmap_sdcpwr:
@@ -204,6 +205,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
out_unmap_sdcasr:
iounmap(sdcasr_mapbase);
out:
+ of_node_put(cpu);
return err;
}
--
2.9.5
^ permalink raw reply related
* [PATCH v11 0/5] Add utilization clamping support (CGroups API)
From: Patrick Bellasi @ 2019-07-08 8:43 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, Rafael J . Wysocki,
Vincent Guittot, Viresh Kumar, Paul Turner, Quentin Perret,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle, Suren Baghdasaryan, Alessio Balsini
Hi all, this is a follow up of:
https://lore.kernel.org/lkml/20190621084217.8167-1-patrick.bellasi@arm.com/
to respin all the bits not yet queued by PeterZ and addressing all Tejun's
requests from previous review:
- remove checks for cpu_uclamp_{min,max}_write()s from root group
- remove checks on "protections" being smaller then "limits"
- rephrase uclamp extension description to avoid explicit
mentioning of the bandwidth concept
the series is based on top of:
tj/cgroup.git for-5.3
tip/tip.git sched/core
I hope this version covers all major details about the expected behavior
and delegation model. The code however can still benefit from a better
review, looking forward for any additional feedback.
Cheers,
Patrick
Series Organization
===================
This series contains just the remaining bits of the original posting:
- Patches [0-5]: Per task group (secondary) API
and it is based on today's tj/cgroup/for-5.3 and tip/sched/core.
The full tree is available here:
git://linux-arm.org/linux-pb.git lkml/utilclamp_v11
http://www.linux-arm.org/git?p=linux-pb.git;a=shortlog;h=refs/heads/lkml/utilclamp_v11
where you can also get the patches already queued in tip/sched/core
- Patches [01-07]: Per task (primary) API
- Patches [08-09]: Schedutil integration for FAIR and RT tasks
- Patches [10-11]: Integration with EAS's energy_compute()
Newcomer's Short Abstract
=========================
The Linux scheduler tracks a "utilization" signal for each scheduling entity
(SE), e.g. tasks, to know how much CPU time they use. This signal allows the
scheduler to know how "big" a task is and, in principle, it can support
advanced task placement strategies by selecting the best CPU to run a task.
Some of these strategies are represented by the Energy Aware Scheduler [1].
When the schedutil cpufreq governor is in use, the utilization signal allows
the Linux scheduler to also drive frequency selection. The CPU utilization
signal, which represents the aggregated utilization of tasks scheduled on that
CPU, is used to select the frequency which best fits the workload generated by
the tasks.
The current translation of utilization values into a frequency selection is
simple: we go to max for RT tasks or to the minimum frequency which can
accommodate the utilization of DL+FAIR tasks.
However, utilization values by themselves cannot convey the desired
power/performance behaviors of each task as intended by user-space.
As such they are not ideally suited for task placement decisions.
Task placement and frequency selection policies in the kernel can be improved
by taking into consideration hints coming from authorized user-space elements,
like for example the Android middleware or more generally any "System
Management Software" (SMS) framework.
Utilization clamping is a mechanism which allows to "clamp" (i.e. filter) the
utilization generated by RT and FAIR tasks within a range defined by user-space.
The clamped utilization value can then be used, for example, to enforce a
minimum and/or maximum frequency depending on which tasks are active on a CPU.
The main use-cases for utilization clamping are:
- boosting: better interactive response for small tasks which
are affecting the user experience.
Consider for example the case of a small control thread for an external
accelerator (e.g. GPU, DSP, other devices). Here, from the task utilization
the scheduler does not have a complete view of what the task's requirements
are and, if it's a small utilization task, it keeps selecting a more energy
efficient CPU, with smaller capacity and lower frequency, thus negatively
impacting the overall time required to complete task activations.
- capping: increase energy efficiency for background tasks not affecting the
user experience.
Since running on a lower capacity CPU at a lower frequency is more energy
efficient, when the completion time is not a main goal, then capping the
utilization considered for certain (maybe big) tasks can have positive
effects, both on energy consumption and thermal headroom.
This feature allows also to make RT tasks more energy friendly on mobile
systems where running them on high capacity CPUs and at the maximum
frequency is not required.
From these two use-cases, it's worth noticing that frequency selection
biasing, introduced by patches 9 and 10 of this series, is just one possible
usage of utilization clamping. Another compelling extension of utilization
clamping is in helping the scheduler in making tasks placement decisions.
Utilization is (also) a task specific property the scheduler uses to know
how much CPU bandwidth a task requires, at least as long as there is idle time.
Thus, the utilization clamp values, defined either per-task or per-task_group,
can represent tasks to the scheduler as being bigger (or smaller) than what
they actually are.
Utilization clamping thus enables interesting additional optimizations, for
example on asymmetric capacity systems like Arm big.LITTLE and DynamIQ CPUs,
where:
- boosting: try to run small/foreground tasks on higher-capacity CPUs to
complete them faster despite being less energy efficient.
- capping: try to run big/background tasks on low-capacity CPUs to save power
and thermal headroom for more important tasks
This series does not present this additional usage of utilization clamping but
it's an integral part of the EAS feature set, where [2] is one of its main
components.
Android kernels use SchedTune, a solution similar to utilization clamping, to
bias both 'frequency selection' and 'task placement'. This series provides the
foundation to add similar features to mainline while focusing, for the
time being, just on schedutil integration.
References
==========
[1] Energy Aware Scheduling
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/scheduler/sched-energy.txt?h=v5.1
[2] Expressing per-task/per-cgroup performance hints
Linux Plumbers Conference 2018
https://linuxplumbersconf.org/event/2/contributions/128/
Patrick Bellasi (5):
sched/core: uclamp: Extend CPU's cgroup controller
sched/core: uclamp: Propagate parent clamps
sched/core: uclamp: Propagate system defaults to root group
sched/core: uclamp: Use TG's clamps to restrict TASK's clamps
sched/core: uclamp: Update CPU's refcount on TG's clamp changes
Documentation/admin-guide/cgroup-v2.rst | 30 +++
init/Kconfig | 22 ++
kernel/sched/core.c | 335 +++++++++++++++++++++++-
kernel/sched/sched.h | 8 +
4 files changed, 392 insertions(+), 3 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH v11 1/5] sched/core: uclamp: Extend CPU's cgroup controller
From: Patrick Bellasi @ 2019-07-08 8:43 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, Rafael J . Wysocki,
Vincent Guittot, Viresh Kumar, Paul Turner, Quentin Perret,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle, Suren Baghdasaryan, Alessio Balsini
In-Reply-To: <20190708084357.12944-1-patrick.bellasi@arm.com>
The cgroup CPU bandwidth controller allows to assign a specified
(maximum) bandwidth to the tasks of a group. However this bandwidth is
defined and enforced only on a temporal base, without considering the
actual frequency a CPU is running on. Thus, the amount of computation
completed by a task within an allocated bandwidth can be very different
depending on the actual frequency the CPU is running that task.
The amount of computation can be affected also by the specific CPU a
task is running on, especially when running on asymmetric capacity
systems like Arm's big.LITTLE.
With the availability of schedutil, the scheduler is now able
to drive frequency selections based on actual task utilization.
Moreover, the utilization clamping support provides a mechanism to
bias the frequency selection operated by schedutil depending on
constraints assigned to the tasks currently RUNNABLE on a CPU.
Giving the mechanisms described above, it is now possible to extend the
cpu controller to specify the minimum (or maximum) utilization which
should be considered for tasks RUNNABLE on a cpu.
This makes it possible to better defined the actual computational
power assigned to task groups, thus improving the cgroup CPU bandwidth
controller which is currently based just on time constraints.
Extend the CPU controller with a couple of new attributes uclamp.{min,max}
which allow to enforce utilization boosting and capping for all the
tasks in a group.
Specifically:
- uclamp.min: defines the minimum utilization which should be considered
i.e. the RUNNABLE tasks of this group will run at least at a
minimum frequency which corresponds to the uclamp.min
utilization
- uclamp.max: defines the maximum utilization which should be considered
i.e. the RUNNABLE tasks of this group will run up to a
maximum frequency which corresponds to the uclamp.max
utilization
These attributes:
a) are available only for non-root nodes, both on default and legacy
hierarchies, while system wide clamps are defined by a generic
interface which does not depends on cgroups. This system wide
interface enforces constraints on tasks in the root node.
b) enforce effective constraints at each level of the hierarchy which
are a restriction of the group requests considering its parent's
effective constraints. Root group effective constraints are defined
by the system wide interface.
This mechanism allows each (non-root) level of the hierarchy to:
- request whatever clamp values it would like to get
- effectively get only up to the maximum amount allowed by its parent
c) have higher priority than task-specific clamps, defined via
sched_setattr(), thus allowing to control and restrict task requests.
Add two new attributes to the cpu controller to collect "requested"
clamp values. Allow that at each non-root level of the hierarchy.
Validate local consistency by enforcing uclamp.min < uclamp.max.
Keep it simple by not caring now about "effective" values computation
and propagation along the hierarchy.
Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
---
Changes in v11:
Message-ID: <20190624175215.GR657710@devbig004.ftw2.facebook.com>
- remove checks for cpu_uclamp_{min,max}_write() from root group
- remove enforcement for "protection" being smaller than "limits"
- rephrase uclamp extension description to avoid explicit
mentioning of the bandwidth concept
---
Documentation/admin-guide/cgroup-v2.rst | 30 +++++
init/Kconfig | 22 ++++
kernel/sched/core.c | 161 +++++++++++++++++++++++-
kernel/sched/sched.h | 6 +
4 files changed, 218 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index a5c845338d6d..1d49426b4c1e 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -951,6 +951,13 @@ controller implements weight and absolute bandwidth limit models for
normal scheduling policy and absolute bandwidth allocation model for
realtime scheduling policy.
+In all the above models, cycles distribution is defined only on a temporal
+base and it does not account for the frequency at which tasks are executed.
+The (optional) utilization clamping support allows to hint the schedutil
+cpufreq governor about the minimum desired frequency which should always be
+provided by a CPU, as well as the maximum desired frequency, which should not
+be exceeded by a CPU.
+
WARNING: cgroup2 doesn't yet support control of realtime processes and
the cpu controller can only be enabled when all RT processes are in
the root cgroup. Be aware that system management software may already
@@ -1016,6 +1023,29 @@ All time durations are in microseconds.
Shows pressure stall information for CPU. See
Documentation/accounting/psi.txt for details.
+ cpu.uclamp.min
+ A read-write single value file which exists on non-root cgroups.
+ The default is "0", i.e. no utilization boosting.
+
+ The requested minimum utilization as a percentage rational number,
+ e.g. 12.34 for 12.34%.
+
+ This interface allows reading and setting minimum utilization clamp
+ values similar to the sched_setattr(2). This minimum utilization
+ value is used to clamp the task specific minimum utilization clamp.
+
+ cpu.uclamp.max
+ A read-write single value file which exists on non-root cgroups.
+ The default is "max". i.e. no utilization capping
+
+ The requested maximum utilization as a percentage rational number,
+ e.g. 98.76 for 98.76%.
+
+ This interface allows reading and setting maximum utilization clamp
+ values similar to the sched_setattr(2). This maximum utilization
+ value is used to clamp the task specific maximum utilization clamp.
+
+
Memory
------
diff --git a/init/Kconfig b/init/Kconfig
index bf96faf3fe43..68a21188786c 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -903,6 +903,28 @@ config RT_GROUP_SCHED
endif #CGROUP_SCHED
+config UCLAMP_TASK_GROUP
+ bool "Utilization clamping per group of tasks"
+ depends on CGROUP_SCHED
+ depends on UCLAMP_TASK
+ default n
+ help
+ This feature enables the scheduler to track the clamped utilization
+ of each CPU based on RUNNABLE tasks currently scheduled on that CPU.
+
+ When this option is enabled, the user can specify a min and max
+ CPU bandwidth which is allowed for each single task in a group.
+ The max bandwidth allows to clamp the maximum frequency a task
+ can use, while the min bandwidth allows to define a minimum
+ frequency a task will always use.
+
+ When task group based utilization clamping is enabled, an eventually
+ specified task-specific clamp value is constrained by the cgroup
+ specified clamp value. Both minimum and maximum task clamping cannot
+ be bigger than the corresponding clamping defined at task group level.
+
+ If in doubt, say N.
+
config CGROUP_PIDS
bool "PIDs controller"
help
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fa43ce3962e7..17ebdaaf7cd9 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1149,8 +1149,12 @@ static void __init init_uclamp(void)
/* System defaults allow max clamp values for both indexes */
uclamp_se_set(&uc_max, uclamp_none(UCLAMP_MAX), false);
- for_each_clamp_id(clamp_id)
+ for_each_clamp_id(clamp_id) {
uclamp_default[clamp_id] = uc_max;
+#ifdef CONFIG_UCLAMP_TASK_GROUP
+ root_task_group.uclamp_req[clamp_id] = uc_max;
+#endif
+ }
}
#else /* CONFIG_UCLAMP_TASK */
@@ -6725,6 +6729,19 @@ void ia64_set_curr_task(int cpu, struct task_struct *p)
/* task_group_lock serializes the addition/removal of task groups */
static DEFINE_SPINLOCK(task_group_lock);
+static inline void alloc_uclamp_sched_group(struct task_group *tg,
+ struct task_group *parent)
+{
+#ifdef CONFIG_UCLAMP_TASK_GROUP
+ int clamp_id;
+
+ for_each_clamp_id(clamp_id) {
+ uclamp_se_set(&tg->uclamp_req[clamp_id],
+ uclamp_none(clamp_id), false);
+ }
+#endif
+}
+
static void sched_free_group(struct task_group *tg)
{
free_fair_sched_group(tg);
@@ -6748,6 +6765,8 @@ struct task_group *sched_create_group(struct task_group *parent)
if (!alloc_rt_sched_group(tg, parent))
goto err;
+ alloc_uclamp_sched_group(tg, parent);
+
return tg;
err:
@@ -6968,6 +6987,118 @@ static void cpu_cgroup_attach(struct cgroup_taskset *tset)
sched_move_task(task);
}
+#ifdef CONFIG_UCLAMP_TASK_GROUP
+static inline int uclamp_scale_from_percent(char *buf, u64 *value)
+{
+ *value = SCHED_CAPACITY_SCALE;
+
+ buf = strim(buf);
+ if (strncmp("max", buf, 4)) {
+ s64 percent;
+ int ret;
+
+ ret = cgroup_parse_float(buf, 2, &percent);
+ if (ret)
+ return ret;
+
+ percent <<= SCHED_CAPACITY_SHIFT;
+ *value = DIV_ROUND_CLOSEST_ULL(percent, 10000);
+ }
+
+ return 0;
+}
+
+static inline u64 uclamp_percent_from_scale(u64 value)
+{
+ return DIV_ROUND_CLOSEST_ULL(value * 10000, SCHED_CAPACITY_SCALE);
+}
+
+static ssize_t cpu_uclamp_min_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes,
+ loff_t off)
+{
+ struct task_group *tg;
+ u64 min_value;
+ int ret;
+
+ ret = uclamp_scale_from_percent(buf, &min_value);
+ if (ret)
+ return ret;
+ if (min_value > SCHED_CAPACITY_SCALE)
+ return -ERANGE;
+
+ rcu_read_lock();
+
+ tg = css_tg(of_css(of));
+ if (tg->uclamp_req[UCLAMP_MIN].value != min_value)
+ uclamp_se_set(&tg->uclamp_req[UCLAMP_MIN], min_value, false);
+
+ rcu_read_unlock();
+
+ return nbytes;
+}
+
+static ssize_t cpu_uclamp_max_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes,
+ loff_t off)
+{
+ struct task_group *tg;
+ u64 max_value;
+ int ret;
+
+ ret = uclamp_scale_from_percent(buf, &max_value);
+ if (ret)
+ return ret;
+ if (max_value > SCHED_CAPACITY_SCALE)
+ return -ERANGE;
+
+ rcu_read_lock();
+
+ tg = css_tg(of_css(of));
+ if (tg->uclamp_req[UCLAMP_MAX].value != max_value)
+ uclamp_se_set(&tg->uclamp_req[UCLAMP_MAX], max_value, false);
+
+ rcu_read_unlock();
+
+ return nbytes;
+}
+
+static inline void cpu_uclamp_print(struct seq_file *sf,
+ enum uclamp_id clamp_id)
+{
+ struct task_group *tg;
+ u64 util_clamp;
+ u64 percent;
+ u32 rem;
+
+ rcu_read_lock();
+ tg = css_tg(seq_css(sf));
+ util_clamp = tg->uclamp_req[clamp_id].value;
+ rcu_read_unlock();
+
+ if (util_clamp == SCHED_CAPACITY_SCALE) {
+ seq_puts(sf, "max\n");
+ return;
+ }
+
+ percent = uclamp_percent_from_scale(util_clamp);
+ percent = div_u64_rem(percent, 100, &rem);
+ seq_printf(sf, "%llu.%u\n", percent, rem);
+}
+
+static int cpu_uclamp_min_show(struct seq_file *sf, void *v)
+{
+ cpu_uclamp_print(sf, UCLAMP_MIN);
+ return 0;
+}
+
+static int cpu_uclamp_max_show(struct seq_file *sf, void *v)
+{
+ cpu_uclamp_print(sf, UCLAMP_MAX);
+ return 0;
+}
+#endif /* CONFIG_UCLAMP_TASK_GROUP */
+
#ifdef CONFIG_FAIR_GROUP_SCHED
static int cpu_shares_write_u64(struct cgroup_subsys_state *css,
struct cftype *cftype, u64 shareval)
@@ -7312,6 +7443,20 @@ static struct cftype cpu_legacy_files[] = {
.read_u64 = cpu_rt_period_read_uint,
.write_u64 = cpu_rt_period_write_uint,
},
+#endif
+#ifdef CONFIG_UCLAMP_TASK_GROUP
+ {
+ .name = "uclamp.min",
+ .flags = CFTYPE_NOT_ON_ROOT,
+ .seq_show = cpu_uclamp_min_show,
+ .write = cpu_uclamp_min_write,
+ },
+ {
+ .name = "uclamp.max",
+ .flags = CFTYPE_NOT_ON_ROOT,
+ .seq_show = cpu_uclamp_max_show,
+ .write = cpu_uclamp_max_write,
+ },
#endif
{ } /* Terminate */
};
@@ -7479,6 +7624,20 @@ static struct cftype cpu_files[] = {
.seq_show = cpu_max_show,
.write = cpu_max_write,
},
+#endif
+#ifdef CONFIG_UCLAMP_TASK_GROUP
+ {
+ .name = "uclamp.min",
+ .flags = CFTYPE_NOT_ON_ROOT,
+ .seq_show = cpu_uclamp_min_show,
+ .write = cpu_uclamp_min_write,
+ },
+ {
+ .name = "uclamp.max",
+ .flags = CFTYPE_NOT_ON_ROOT,
+ .seq_show = cpu_uclamp_max_show,
+ .write = cpu_uclamp_max_write,
+ },
#endif
{ } /* terminate */
};
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 802b1f3405f2..3723037ea80d 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -393,6 +393,12 @@ struct task_group {
#endif
struct cfs_bandwidth cfs_bandwidth;
+
+#ifdef CONFIG_UCLAMP_TASK_GROUP
+ /* Clamp values requested for a task group */
+ struct uclamp_se uclamp_req[UCLAMP_CNT];
+#endif
+
};
#ifdef CONFIG_FAIR_GROUP_SCHED
--
2.21.0
^ permalink raw reply related
* [PATCH v11 3/5] sched/core: uclamp: Propagate system defaults to root group
From: Patrick Bellasi @ 2019-07-08 8:43 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, Rafael J . Wysocki,
Vincent Guittot, Viresh Kumar, Paul Turner, Quentin Perret,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle, Suren Baghdasaryan, Alessio Balsini
In-Reply-To: <20190708084357.12944-1-patrick.bellasi@arm.com>
The clamp values are not tunable at the level of the root task group.
That's for two main reasons:
- the root group represents "system resources" which are always
entirely available from the cgroup standpoint.
- when tuning/restricting "system resources" makes sense, tuning must
be done using a system wide API which should also be available when
control groups are not.
When a system wide restriction is available, cgroups should be aware of
its value in order to know exactly how much "system resources" are
available for the subgroups.
Utilization clamping supports already the concepts of:
- system defaults: which define the maximum possible clamp values
usable by tasks.
- effective clamps: which allows a parent cgroup to constraint (maybe
temporarily) its descendants without losing the information related
to the values "requested" from them.
Exploit these two concepts and bind them together in such a way that,
whenever system default are tuned, the new values are propagated to
(possibly) restrict or relax the "effective" value of nested cgroups.
Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
---
kernel/sched/core.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ec91f4518752..276f9c2f6103 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1017,12 +1017,30 @@ static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p)
uclamp_rq_dec_id(rq, p, clamp_id);
}
+#ifdef CONFIG_UCLAMP_TASK_GROUP
+static void cpu_util_update_eff(struct cgroup_subsys_state *css);
+static void uclamp_update_root_tg(void)
+{
+ struct task_group *tg = &root_task_group;
+
+ uclamp_se_set(&tg->uclamp_req[UCLAMP_MIN],
+ sysctl_sched_uclamp_util_min, false);
+ uclamp_se_set(&tg->uclamp_req[UCLAMP_MAX],
+ sysctl_sched_uclamp_util_max, false);
+
+ cpu_util_update_eff(&root_task_group.css);
+}
+#else
+static void uclamp_update_root_tg(void) { }
+#endif
+
int sysctl_sched_uclamp_handler(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp,
loff_t *ppos)
{
- int old_min, old_max;
+ bool update_root_tg = false;
static DEFINE_MUTEX(mutex);
+ int old_min, old_max;
int result;
mutex_lock(&mutex);
@@ -1044,12 +1062,17 @@ int sysctl_sched_uclamp_handler(struct ctl_table *table, int write,
if (old_min != sysctl_sched_uclamp_util_min) {
uclamp_se_set(&uclamp_default[UCLAMP_MIN],
sysctl_sched_uclamp_util_min, false);
+ update_root_tg = true;
}
if (old_max != sysctl_sched_uclamp_util_max) {
uclamp_se_set(&uclamp_default[UCLAMP_MAX],
sysctl_sched_uclamp_util_max, false);
+ update_root_tg = true;
}
+ if (update_root_tg)
+ uclamp_update_root_tg();
+
/*
* Updating all the RUNNABLE task is expensive, keep it simple and do
* just a lazy update at each next enqueue time.
--
2.21.0
^ permalink raw reply related
* [PATCH v11 4/5] sched/core: uclamp: Use TG's clamps to restrict TASK's clamps
From: Patrick Bellasi @ 2019-07-08 8:43 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, Rafael J . Wysocki,
Vincent Guittot, Viresh Kumar, Paul Turner, Quentin Perret,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle, Suren Baghdasaryan, Alessio Balsini
In-Reply-To: <20190708084357.12944-1-patrick.bellasi@arm.com>
When a task specific clamp value is configured via sched_setattr(2), this
value is accounted in the corresponding clamp bucket every time the task is
{en,de}qeued. However, when cgroups are also in use, the task specific
clamp values could be restricted by the task_group (TG) clamp values.
Update uclamp_cpu_inc() to aggregate task and TG clamp values. Every time a
task is enqueued, it's accounted in the clamp bucket tracking the smaller
clamp between the task specific value and its TG effective value. This
allows to:
1. ensure cgroup clamps are always used to restrict task specific requests,
i.e. boosted not more than its TG effective protection and capped at
least as its TG effective limit.
2. implement a "nice-like" policy, where tasks are still allowed to request
less than what enforced by their TG effective limits and protections
This mimics what already happens for a task's CPU affinity mask when the
task is also in a cpuset, i.e. cgroup attributes are always used to
restrict per-task attributes.
Do this by exploiting the concept of "effective" clamp, which is already
used by a TG to track parent enforced restrictions.
Apply task group clamp restrictions only to tasks belonging to a child
group. While, for tasks in the root group or in an autogroup, only system
defaults are enforced.
Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
---
kernel/sched/core.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 276f9c2f6103..2591a70c85cf 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -873,16 +873,42 @@ unsigned int uclamp_rq_max_value(struct rq *rq, unsigned int clamp_id,
return uclamp_idle_value(rq, clamp_id, clamp_value);
}
+static inline struct uclamp_se
+uclamp_tg_restrict(struct task_struct *p, unsigned int clamp_id)
+{
+ struct uclamp_se uc_req = p->uclamp_req[clamp_id];
+#ifdef CONFIG_UCLAMP_TASK_GROUP
+ struct uclamp_se uc_max;
+
+ /*
+ * Tasks in autogroups or root task group will be
+ * restricted by system defaults.
+ */
+ if (task_group_is_autogroup(task_group(p)))
+ return uc_req;
+ if (task_group(p) == &root_task_group)
+ return uc_req;
+
+ uc_max = task_group(p)->uclamp[clamp_id];
+ if (uc_req.value > uc_max.value || !uc_req.user_defined)
+ return uc_max;
+#endif
+
+ return uc_req;
+}
+
/*
* The effective clamp bucket index of a task depends on, by increasing
* priority:
* - the task specific clamp value, when explicitly requested from userspace
+ * - the task group effective clamp value, for tasks not either in the root
+ * group or in an autogroup
* - the system default clamp value, defined by the sysadmin
*/
static inline struct uclamp_se
uclamp_eff_get(struct task_struct *p, unsigned int clamp_id)
{
- struct uclamp_se uc_req = p->uclamp_req[clamp_id];
+ struct uclamp_se uc_req = uclamp_tg_restrict(p, clamp_id);
struct uclamp_se uc_max = uclamp_default[clamp_id];
/* System default restrictions always apply */
--
2.21.0
^ permalink raw reply related
* [PATCH v11 5/5] sched/core: uclamp: Update CPU's refcount on TG's clamp changes
From: Patrick Bellasi @ 2019-07-08 8:43 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, Rafael J . Wysocki,
Vincent Guittot, Viresh Kumar, Paul Turner, Quentin Perret,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle, Suren Baghdasaryan, Alessio Balsini
In-Reply-To: <20190708084357.12944-1-patrick.bellasi@arm.com>
On updates of task group (TG) clamp values, ensure that these new values
are enforced on all RUNNABLE tasks of the task group, i.e. all RUNNABLE
tasks are immediately boosted and/or capped as requested.
Do that each time we update effective clamps from cpu_util_update_eff().
Use the *cgroup_subsys_state (css) to walk the list of tasks in each
affected TG and update their RUNNABLE tasks.
Update each task by using the same mechanism used for cpu affinity masks
updates, i.e. by taking the rq lock.
Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
---
Changes in v11:
Message-ID: <20190624174607.GQ657710@devbig004.ftw2.facebook.com>
- Ensure group limits always clamps group protection
---
kernel/sched/core.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2591a70c85cf..ddc5fcd4b9cf 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1043,6 +1043,57 @@ static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p)
uclamp_rq_dec_id(rq, p, clamp_id);
}
+static inline void
+uclamp_update_active(struct task_struct *p, unsigned int clamp_id)
+{
+ struct rq_flags rf;
+ struct rq *rq;
+
+ /*
+ * Lock the task and the rq where the task is (or was) queued.
+ *
+ * We might lock the (previous) rq of a !RUNNABLE task, but that's the
+ * price to pay to safely serialize util_{min,max} updates with
+ * enqueues, dequeues and migration operations.
+ * This is the same locking schema used by __set_cpus_allowed_ptr().
+ */
+ rq = task_rq_lock(p, &rf);
+
+ /*
+ * Setting the clamp bucket is serialized by task_rq_lock().
+ * If the task is not yet RUNNABLE and its task_struct is not
+ * affecting a valid clamp bucket, the next time it's enqueued,
+ * it will already see the updated clamp bucket value.
+ */
+ if (!p->uclamp[clamp_id].active)
+ goto done;
+
+ uclamp_rq_dec_id(rq, p, clamp_id);
+ uclamp_rq_inc_id(rq, p, clamp_id);
+
+done:
+
+ task_rq_unlock(rq, p, &rf);
+}
+
+static inline void
+uclamp_update_active_tasks(struct cgroup_subsys_state *css,
+ unsigned int clamps)
+{
+ struct css_task_iter it;
+ struct task_struct *p;
+ unsigned int clamp_id;
+
+ css_task_iter_start(css, 0, &it);
+ while ((p = css_task_iter_next(&it))) {
+ for_each_clamp_id(clamp_id) {
+ if ((0x1 << clamp_id) & clamps)
+ uclamp_update_active(p, clamp_id);
+ }
+ }
+ css_task_iter_end(&it);
+}
+
#ifdef CONFIG_UCLAMP_TASK_GROUP
static void cpu_util_update_eff(struct cgroup_subsys_state *css);
static void uclamp_update_root_tg(void)
@@ -7087,8 +7138,13 @@ static void cpu_util_update_eff(struct cgroup_subsys_state *css)
uc_se[clamp_id].bucket_id = uclamp_bucket_id(eff[clamp_id]);
clamps |= (0x1 << clamp_id);
}
- if (!clamps)
+ if (!clamps) {
css = css_rightmost_descendant(css);
+ continue;
+ }
+
+ /* Immediately update descendants RUNNABLE tasks */
+ uclamp_update_active_tasks(css, clamps);
}
}
--
2.21.0
^ permalink raw reply related
* [PATCH v11 2/5] sched/core: uclamp: Propagate parent clamps
From: Patrick Bellasi @ 2019-07-08 8:43 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, Rafael J . Wysocki,
Vincent Guittot, Viresh Kumar, Paul Turner, Quentin Perret,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle, Suren Baghdasaryan, Alessio Balsini
In-Reply-To: <20190708084357.12944-1-patrick.bellasi@arm.com>
In order to properly support hierarchical resources control, the cgroup
delegation model requires that attribute writes from a child group never
fail but still are locally consistent and constrained based on parent's
assigned resources. This requires to properly propagate and aggregate
parent attributes down to its descendants.
Implement this mechanism by adding a new "effective" clamp value for each
task group. The effective clamp value is defined as the smaller value
between the clamp value of a group and the effective clamp value of its
parent. This is the actual clamp value enforced on tasks in a task group.
Since it's possible for a cpu.uclamp.min value to be bigger than the
cpu.uclamp.max value, ensure local consistency by restricting each
"protection"
(i.e. min utilization) with the corresponding "limit" (i.e. max
utilization). Do that at effective clamps propagation to ensure all
user-space write never fails while still always tracking the most
restrictive values.
Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
---
Changes in v11:
Message-ID: <20190624174607.GQ657710@devbig004.ftw2.facebook.com>
- Removed user-space uclamp.{min.max}.effective API
- Ensure group limits always clamps group protections
---
kernel/sched/core.c | 65 ++++++++++++++++++++++++++++++++++++++++++++
kernel/sched/sched.h | 2 ++
2 files changed, 67 insertions(+)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 17ebdaaf7cd9..ec91f4518752 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -773,6 +773,18 @@ static void set_load_weight(struct task_struct *p, bool update_load)
}
#ifdef CONFIG_UCLAMP_TASK
+/*
+ * Serializes updates of utilization clamp values
+ *
+ * The (slow-path) user-space triggers utilization clamp value updates which
+ * can require updates on (fast-path) scheduler's data structures used to
+ * support enqueue/dequeue operations.
+ * While the per-CPU rq lock protects fast-path update operations, user-space
+ * requests are serialized using a mutex to reduce the risk of conflicting
+ * updates or API abuses.
+ */
+static DEFINE_MUTEX(uclamp_mutex);
+
/* Max allowed minimum utilization */
unsigned int sysctl_sched_uclamp_util_min = SCHED_CAPACITY_SCALE;
@@ -1137,6 +1149,8 @@ static void __init init_uclamp(void)
unsigned int clamp_id;
int cpu;
+ mutex_init(&uclamp_mutex);
+
for_each_possible_cpu(cpu) {
memset(&cpu_rq(cpu)->uclamp, 0, sizeof(struct uclamp_rq));
cpu_rq(cpu)->uclamp_flags = 0;
@@ -1153,6 +1167,7 @@ static void __init init_uclamp(void)
uclamp_default[clamp_id] = uc_max;
#ifdef CONFIG_UCLAMP_TASK_GROUP
root_task_group.uclamp_req[clamp_id] = uc_max;
+ root_task_group.uclamp[clamp_id] = uc_max;
#endif
}
}
@@ -6738,6 +6753,7 @@ static inline void alloc_uclamp_sched_group(struct task_group *tg,
for_each_clamp_id(clamp_id) {
uclamp_se_set(&tg->uclamp_req[clamp_id],
uclamp_none(clamp_id), false);
+ tg->uclamp[clamp_id] = parent->uclamp[clamp_id];
}
#endif
}
@@ -6988,6 +7004,45 @@ static void cpu_cgroup_attach(struct cgroup_taskset *tset)
}
#ifdef CONFIG_UCLAMP_TASK_GROUP
+static void cpu_util_update_eff(struct cgroup_subsys_state *css)
+{
+ struct cgroup_subsys_state *top_css = css;
+ struct uclamp_se *uc_se = NULL;
+ unsigned int eff[UCLAMP_CNT];
+ unsigned int clamp_id;
+ unsigned int clamps;
+
+ css_for_each_descendant_pre(css, top_css) {
+ uc_se = css_tg(css)->parent
+ ? css_tg(css)->parent->uclamp : NULL;
+
+ for_each_clamp_id(clamp_id) {
+ /* Assume effective clamps matches requested clamps */
+ eff[clamp_id] = css_tg(css)->uclamp_req[clamp_id].value;
+ /* Cap effective clamps with parent's effective clamps */
+ if (uc_se &&
+ eff[clamp_id] > uc_se[clamp_id].value) {
+ eff[clamp_id] = uc_se[clamp_id].value;
+ }
+ }
+ /* Ensure protection is always capped by limit */
+ eff[UCLAMP_MIN] = min(eff[UCLAMP_MIN], eff[UCLAMP_MAX]);
+
+ /* Propagate most restrictive effective clamps */
+ clamps = 0x0;
+ uc_se = css_tg(css)->uclamp;
+ for_each_clamp_id(clamp_id) {
+ if (eff[clamp_id] == uc_se[clamp_id].value)
+ continue;
+ uc_se[clamp_id].value = eff[clamp_id];
+ uc_se[clamp_id].bucket_id = uclamp_bucket_id(eff[clamp_id]);
+ clamps |= (0x1 << clamp_id);
+ }
+ if (!clamps)
+ css = css_rightmost_descendant(css);
+ }
+}
+
static inline int uclamp_scale_from_percent(char *buf, u64 *value)
{
*value = SCHED_CAPACITY_SCALE;
@@ -7027,13 +7082,18 @@ static ssize_t cpu_uclamp_min_write(struct kernfs_open_file *of,
if (min_value > SCHED_CAPACITY_SCALE)
return -ERANGE;
+ mutex_lock(&uclamp_mutex);
rcu_read_lock();
tg = css_tg(of_css(of));
if (tg->uclamp_req[UCLAMP_MIN].value != min_value)
uclamp_se_set(&tg->uclamp_req[UCLAMP_MIN], min_value, false);
+ /* Update effective clamps to track the most restrictive value */
+ cpu_util_update_eff(of_css(of));
+
rcu_read_unlock();
+ mutex_unlock(&uclamp_mutex);
return nbytes;
}
@@ -7052,13 +7112,18 @@ static ssize_t cpu_uclamp_max_write(struct kernfs_open_file *of,
if (max_value > SCHED_CAPACITY_SCALE)
return -ERANGE;
+ mutex_lock(&uclamp_mutex);
rcu_read_lock();
tg = css_tg(of_css(of));
if (tg->uclamp_req[UCLAMP_MAX].value != max_value)
uclamp_se_set(&tg->uclamp_req[UCLAMP_MAX], max_value, false);
+ /* Update effective clamps to track the most restrictive value */
+ cpu_util_update_eff(of_css(of));
+
rcu_read_unlock();
+ mutex_unlock(&uclamp_mutex);
return nbytes;
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 3723037ea80d..8c3aefdaf0ef 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -397,6 +397,8 @@ struct task_group {
#ifdef CONFIG_UCLAMP_TASK_GROUP
/* Clamp values requested for a task group */
struct uclamp_se uclamp_req[UCLAMP_CNT];
+ /* Effective clamp values used for a task group */
+ struct uclamp_se uclamp[UCLAMP_CNT];
#endif
};
--
2.21.0
^ permalink raw reply related
* [PATCH] cpufreq: schedutil: Fix covert rate_limit_us to freq_update_delay_ns overflow
From: ZhangXiaoxu @ 2019-07-08 8:46 UTC (permalink / raw)
To: rjw, viresh.kumar, mingo, peterz, linux-pm, zhangxiaoxu5
When covert rate_limit_us to freq_update_delay_ns, it maybe overflow
and lead an undefined behavior.
So, limit the rate_limit_us to UINT_MAX / NSEC_PER_USEC.
Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
---
kernel/sched/cpufreq_schedutil.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 962cf34..01e05f3 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -590,6 +590,9 @@ rate_limit_us_store(struct gov_attr_set *attr_set, const char *buf, size_t count
if (kstrtouint(buf, 10, &rate_limit_us))
return -EINVAL;
+ if (rate_limit_us > UINT_MAX / NSEC_PER_USEC)
+ return -EINVAL;
+
tunables->rate_limit_us = rate_limit_us;
list_for_each_entry(sg_policy, &attr_set->policy_list, tunables_hook)
--
2.7.4
^ permalink raw reply related
* Re: cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60
From: Rafael J. Wysocki @ 2019-07-08 8:28 UTC (permalink / raw)
To: Viresh Kumar
Cc: Pavel Machek, Rafael J. Wysocki, Matthias Kaehlcke, Ulf Hansson,
Rafael J. Wysocki, Linux-pm mailing list, kernel list,
Stephen Rothwell
In-Reply-To: <20190708030505.kvrg6sh6bd4xzzwa@vireshk-i7>
On Mon, Jul 8, 2019 at 5:05 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 06-07-19, 22:30, Pavel Machek wrote:
> > Hi!
> >
> > > Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
> > > to happen for you.
> > >
> > > I would try
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244
> > >
> > > to narrow down the scope somewhat.
>
> I couldn't find the original mail, what exactly is the problem with
> suspend in your case ?
Something unusual:
https://lore.kernel.org/linux-pm/20190706190123.GA11603@amd/T/#mca22dd7c1e8836e9253702df9f56a68ab65192a4
> > Bisect says:
> >
> > 572542c81dec533b7dd3778ea9f5949a00595f68 is the first bad commit
> > Author: Viresh Kumar <viresh.kumar@linaro.org>
> >
> > cpufreq: Register notifiers with the PM QoS framework
> >
> > This registers the notifiers for min/max frequency constraints
> > with the
> >
> > Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> >
> > Unfortunately, it does not revert cleanly:
>
> I tried following on my ARM board (both single policy and multiple
> policy configurations):
>
> rtcwake --seconds 5 -v -m mem
>
> And everything worked as expected. Please make sure the top commit of
> my series in pm/linux-next is, some issues were fixed on Friday:
>
> 0a811974f3f7 cpufreq: Add QoS requests for userspace constraints
Pavel has tested the latest version of the patch series AFAICS.
The locking added by the commit in question to
refresh_frequency_limits() requires an update of
cpufreq_update_policy(), or it will deadlock in there because of the
lock acquired by cpufreq_cpu_get() if I haven't missed anything.
^ permalink raw reply
* [PATCH] cpufreq: imx-cpufreq-dt: Assign max supported frequency as suspend frequency
From: Anson.Huang @ 2019-07-08 7:46 UTC (permalink / raw)
To: rjw, viresh.kumar, shawnguo, s.hauer, kernel, festevam, linux-pm,
linux-arm-kernel, linux-kernel
Cc: Linux-imx
From: Anson Huang <Anson.Huang@nxp.com>
To reduce the suspend/resume latency, CPU's max supported frequency
should be used during low level suspend/resume phase, "opp-suspend"
property is NOT feasible since OPP defined in DT could be NOT supported
according to speed garding and market segment fuse settings. So we
can assign the cpufreq policy's suspend_freq with max available
frequency provided by cpufreq driver.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/cpufreq/imx-cpufreq-dt.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/cpufreq/imx-cpufreq-dt.c b/drivers/cpufreq/imx-cpufreq-dt.c
index 4f85f31..b6607e8 100644
--- a/drivers/cpufreq/imx-cpufreq-dt.c
+++ b/drivers/cpufreq/imx-cpufreq-dt.c
@@ -4,6 +4,7 @@
*/
#include <linux/cpu.h>
+#include <linux/cpufreq.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/kernel.h>
@@ -84,6 +85,16 @@ static int imx_cpufreq_dt_remove(struct platform_device *pdev)
return 0;
}
+static int __init imx_cpufreq_dt_setup_suspend_opp(void)
+{
+ struct cpufreq_policy *policy = cpufreq_cpu_get(0);
+
+ policy->suspend_freq = cpufreq_quick_get_max(0);
+
+ return 0;
+}
+late_initcall(imx_cpufreq_dt_setup_suspend_opp);
+
static struct platform_driver imx_cpufreq_dt_driver = {
.probe = imx_cpufreq_dt_probe,
.remove = imx_cpufreq_dt_remove,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()
From: Viresh Kumar @ 2019-07-08 7:44 UTC (permalink / raw)
To: Wen Yang
Cc: linux-kernel, xue.zhihong, wang.yi59, cheng.shengyu,
Rafael J. Wysocki, linuxppc-dev, linux-pm
In-Reply-To: <1562570393-8684-1-git-send-email-wen.yang99@zte.com.cn>
On 08-07-19, 15:19, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
>
> Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> v2: clean up the code according to the advice of viresh.
>
> drivers/cpufreq/pasemi-cpufreq.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> index 6b1e4ab..c6d464b 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -128,20 +128,18 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> int cur_astate, idx;
> struct resource res;
> struct device_node *cpu, *dn;
> - int err = -ENODEV;
> + int err;
>
> cpu = of_get_cpu_node(policy->cpu, NULL);
> -
> - of_node_put(cpu);
> if (!cpu)
> - goto out;
> + return -ENODEV;
>
> dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
> if (!dn)
> dn = of_find_compatible_node(NULL, NULL,
> "pasemi,pwrficient-sdc");
> if (!dn)
> - goto out;
> + return -ENODEV;
This change looks incorrect. You still need to drop reference to cpu ?
> err = of_address_to_resource(dn, 0, &res);
> of_node_put(dn);
> if (err)
> @@ -196,6 +194,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> policy->cur = pas_freqs[cur_astate].frequency;
> ppc_proc_freq = policy->cur * 1000ul;
>
> + of_node_put(cpu);
> return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
>
> out_unmap_sdcpwr:
> @@ -204,6 +203,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> out_unmap_sdcasr:
> iounmap(sdcasr_mapbase);
> out:
> + of_node_put(cpu);
> return err;
> }
>
> --
> 2.9.5
--
viresh
^ permalink raw reply
* [PATCH v2] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()
From: Wen Yang @ 2019-07-08 7:19 UTC (permalink / raw)
To: linux-kernel
Cc: xue.zhihong, wang.yi59, cheng.shengyu, Wen Yang,
Rafael J. Wysocki, Viresh Kumar, linuxppc-dev, linux-pm
The cpu variable is still being used in the of_get_property() call
after the of_node_put() call, which may result in use-after-free.
Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
v2: clean up the code according to the advice of viresh.
drivers/cpufreq/pasemi-cpufreq.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
index 6b1e4ab..c6d464b 100644
--- a/drivers/cpufreq/pasemi-cpufreq.c
+++ b/drivers/cpufreq/pasemi-cpufreq.c
@@ -128,20 +128,18 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
int cur_astate, idx;
struct resource res;
struct device_node *cpu, *dn;
- int err = -ENODEV;
+ int err;
cpu = of_get_cpu_node(policy->cpu, NULL);
-
- of_node_put(cpu);
if (!cpu)
- goto out;
+ return -ENODEV;
dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
if (!dn)
dn = of_find_compatible_node(NULL, NULL,
"pasemi,pwrficient-sdc");
if (!dn)
- goto out;
+ return -ENODEV;
err = of_address_to_resource(dn, 0, &res);
of_node_put(dn);
if (err)
@@ -196,6 +194,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
policy->cur = pas_freqs[cur_astate].frequency;
ppc_proc_freq = policy->cur * 1000ul;
+ of_node_put(cpu);
return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
out_unmap_sdcpwr:
@@ -204,6 +203,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
out_unmap_sdcasr:
iounmap(sdcasr_mapbase);
out:
+ of_node_put(cpu);
return err;
}
--
2.9.5
^ permalink raw reply related
* Re: [PATCH V2 00/13] intel_rapl: RAPL abstraction and MMIO RAPL support
From: Zhang Rui @ 2019-07-08 6:53 UTC (permalink / raw)
To: Rafael J. Wysocki, Pandruvada, Srinivas
Cc: linux-pm@vger.kernel.org, rjw@rjwysocki.net
In-Reply-To: <CAJZ5v0ixFF-_5yYZhWBJLUaE6oreaGY1BELGTsYfqYw3M9sDcQ@mail.gmail.com>
On 六, 2019-07-06 at 10:19 +0200, Rafael J. Wysocki wrote:
> On Fri, Jul 5, 2019 at 4:59 PM Pandruvada, Srinivas
> <srinivas.pandruvada@intel.com> wrote:
> >
> >
> > On Fri, 2019-07-05 at 10:57 +0200, Rafael J. Wysocki wrote:
> > >
> > > On Thu, Jul 4, 2019 at 6:34 PM Zhang Rui <rui.zhang@intel.com>
> > > wrote:
> > > >
> > > >
> > > > Besideis MSR interface, RAPL can also be controlled via the
> > > > MMIO
> > > > interface,
> > > > by accessing the MCHBar registers exposed by the processor
> > > > thermal
> > > > device.
> > > >
> > > > Currently, we only have RAPL MSR interface in Linux kernel,
> > > > this
> > > > brings
> > > > problems on some platforms that BIOS performs a low power
> > > > limits
> > > > via the
> > > > MMIO interface by default. This results in poor system
> > > > performance,
> > > > and there is no way for us to change the MMIO MSR setting in
> > > > Linux.
> > > >
> > > > To fix this, RAPL MMIO interface support is introduced in this
> > > > patch set.
> > > >
> > > > Patch 1/13 to patch 11/13 abstract the RAPL code, and move all
> > > > the
> > > > shared
> > > > code into a separate file, intel_rapl_common.c, so that it can
> > > > be
> > > > used
> > > > by both MSR and MMIO interfaces.
> > > > Patch 12/13 introduced RAPL support via MMIO registers, exposed
> > > > by
> > > > the
> > > > processor thermal devices.
> > > > Patch 13/13 fixes a module autoloading issue found later.
> > > >
> > > > The patch series has been tested on Dell XPS 9360, a SKL
> > > > platform.
> > > >
> > > > Note that this patch series are based on the -tip tree, which
> > > > contains the
> > > > latest RAPL changes for multi-die support.
> > > >
> > > > Changes in V2:
> > > > - add kerneldoc for struct rapl_if_priv.
> > > > - use intel_rapl_msr.c for RAPL MSR I/F driver, instead of
> > > > intel_rapl.c.
> > > > - changelog and coding style update.
> > > What tree is the series against?
> > >
> > > It doesn't apply either on top of my powercap branch or on top of
> > > 5.2-rc7 for me.
> > This needs linux tip tree. There are some package/die changes in
> > tip
> > tree, which this patch depends on.
> OK, so the changes in -tip need to go in first.
>
exactly.
BTW, this patch set also conflicts with the RAPL support patches for
icelake platforms. Thus IMO, having a separate rapl branch, and apply
the icl rapl patches on top of this patch set will be much easier.
thanks,
rui
^ permalink raw reply
* Re: [PATCH 04/13] cpufreq: qcom: Refactor the driver to make it easier to extend
From: Ilia Lin @ 2019-07-08 6:30 UTC (permalink / raw)
To: Niklas Cassel
Cc: Andy Gross, Ilia Lin, Rafael J. Wysocki, Viresh Kumar,
linux-arm-msm, jorge.ramirez-ortiz, Stephen Boyd, Viresh Kumar,
bjorn.andersson, ulf.hansson, linux-pm, linux-kernel
In-Reply-To: <20190705095726.21433-5-niklas.cassel@linaro.org>
Reviewed-by: Ilia Lin <ilia.lin@kernel.org>
On Fri, Jul 5, 2019 at 12:58 PM Niklas Cassel <niklas.cassel@linaro.org> wrote:
>
> Refactor the driver to make it easier to extend in a later commit.
>
> Create a driver struct to collect all common resources, in order to make
> it easier to free up all common resources.
> Create a driver match_data struct to make it easier to extend the driver
> with support for new features that might only be supported on certain SoCs.
>
> Co-developed-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
> Changes since RFC:
> -Changed type of versions to u32 from u32*.
> -Make the driver use a match_data struct, so that different SoC can have
> different features.
> -Fixed error handling.
>
> drivers/cpufreq/qcom-cpufreq-nvmem.c | 123 +++++++++++++++++----------
> 1 file changed, 79 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index fad6509eecb5..c0377b0eb2f4 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -43,6 +43,20 @@ enum _msm8996_version {
> NUM_OF_MSM8996_VERSIONS,
> };
>
> +struct qcom_cpufreq_drv;
> +
> +struct qcom_cpufreq_match_data {
> + int (*get_version)(struct device *cpu_dev,
> + struct nvmem_cell *speedbin_nvmem,
> + struct qcom_cpufreq_drv *drv);
> +};
> +
> +struct qcom_cpufreq_drv {
> + struct opp_table **opp_tables;
> + u32 versions;
> + const struct qcom_cpufreq_match_data *data;
> +};
> +
> static struct platform_device *cpufreq_dt_pdev, *cpufreq_pdev;
>
> static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
> @@ -76,7 +90,7 @@ static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
>
> static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
> struct nvmem_cell *speedbin_nvmem,
> - u32 *versions)
> + struct qcom_cpufreq_drv *drv)
> {
> size_t len;
> u8 *speedbin;
> @@ -94,10 +108,10 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
>
> switch (msm8996_version) {
> case MSM8996_V3:
> - *versions = 1 << (unsigned int)(*speedbin);
> + drv->versions = 1 << (unsigned int)(*speedbin);
> break;
> case MSM8996_SG:
> - *versions = 1 << ((unsigned int)(*speedbin) + 4);
> + drv->versions = 1 << ((unsigned int)(*speedbin) + 4);
> break;
> default:
> BUG();
> @@ -108,17 +122,17 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
> return 0;
> }
>
> +static const struct qcom_cpufreq_match_data match_data_kryo = {
> + .get_version = qcom_cpufreq_kryo_name_version,
> +};
> +
> static int qcom_cpufreq_probe(struct platform_device *pdev)
> {
> - struct opp_table **opp_tables;
> - int (*get_version)(struct device *cpu_dev,
> - struct nvmem_cell *speedbin_nvmem,
> - u32 *versions);
> + struct qcom_cpufreq_drv *drv;
> struct nvmem_cell *speedbin_nvmem;
> struct device_node *np;
> struct device *cpu_dev;
> unsigned cpu;
> - u32 versions;
> const struct of_device_id *match;
> int ret;
>
> @@ -126,11 +140,6 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
> if (!cpu_dev)
> return -ENODEV;
>
> - match = pdev->dev.platform_data;
> - get_version = match->data;
> - if (!get_version)
> - return -ENODEV;
> -
> np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> if (!np)
> return -ENOENT;
> @@ -141,23 +150,43 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
> return -ENOENT;
> }
>
> - speedbin_nvmem = of_nvmem_cell_get(np, NULL);
> - of_node_put(np);
> - if (IS_ERR(speedbin_nvmem)) {
> - if (PTR_ERR(speedbin_nvmem) != -EPROBE_DEFER)
> - dev_err(cpu_dev, "Could not get nvmem cell: %ld\n",
> - PTR_ERR(speedbin_nvmem));
> - return PTR_ERR(speedbin_nvmem);
> + drv = kzalloc(sizeof(*drv), GFP_KERNEL);
> + if (!drv)
> + return -ENOMEM;
> +
> + match = pdev->dev.platform_data;
> + drv->data = match->data;
> + if (!drv->data) {
> + ret = -ENODEV;
> + goto free_drv;
> }
>
> - ret = get_version(cpu_dev, speedbin_nvmem, &versions);
> - nvmem_cell_put(speedbin_nvmem);
> - if (ret)
> - return ret;
> + if (drv->data->get_version) {
> + speedbin_nvmem = of_nvmem_cell_get(np, NULL);
> + of_node_put(np);
> + if (IS_ERR(speedbin_nvmem)) {
> + if (PTR_ERR(speedbin_nvmem) != -EPROBE_DEFER)
> + dev_err(cpu_dev,
> + "Could not get nvmem cell: %ld\n",
> + PTR_ERR(speedbin_nvmem));
> + ret = PTR_ERR(speedbin_nvmem);
> + goto free_drv;
> + }
>
> - opp_tables = kcalloc(num_possible_cpus(), sizeof(*opp_tables), GFP_KERNEL);
> - if (!opp_tables)
> - return -ENOMEM;
> + ret = drv->data->get_version(cpu_dev, speedbin_nvmem, drv);
> + if (ret) {
> + nvmem_cell_put(speedbin_nvmem);
> + goto free_drv;
> + }
> + nvmem_cell_put(speedbin_nvmem);
> + }
> +
> + drv->opp_tables = kcalloc(num_possible_cpus(), sizeof(*drv->opp_tables),
> + GFP_KERNEL);
> + if (!drv->opp_tables) {
> + ret = -ENOMEM;
> + goto free_drv;
> + }
>
> for_each_possible_cpu(cpu) {
> cpu_dev = get_cpu_device(cpu);
> @@ -166,19 +195,23 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
> goto free_opp;
> }
>
> - opp_tables[cpu] = dev_pm_opp_set_supported_hw(cpu_dev,
> - &versions, 1);
> - if (IS_ERR(opp_tables[cpu])) {
> - ret = PTR_ERR(opp_tables[cpu]);
> - dev_err(cpu_dev, "Failed to set supported hardware\n");
> - goto free_opp;
> + if (drv->data->get_version) {
> + drv->opp_tables[cpu] =
> + dev_pm_opp_set_supported_hw(cpu_dev,
> + &drv->versions, 1);
> + if (IS_ERR(drv->opp_tables[cpu])) {
> + ret = PTR_ERR(drv->opp_tables[cpu]);
> + dev_err(cpu_dev,
> + "Failed to set supported hardware\n");
> + goto free_opp;
> + }
> }
> }
>
> cpufreq_dt_pdev = platform_device_register_simple("cpufreq-dt", -1,
> NULL, 0);
> if (!IS_ERR(cpufreq_dt_pdev)) {
> - platform_set_drvdata(pdev, opp_tables);
> + platform_set_drvdata(pdev, drv);
> return 0;
> }
>
> @@ -187,26 +220,30 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
>
> free_opp:
> for_each_possible_cpu(cpu) {
> - if (IS_ERR_OR_NULL(opp_tables[cpu]))
> + if (IS_ERR_OR_NULL(drv->opp_tables[cpu]))
> break;
> - dev_pm_opp_put_supported_hw(opp_tables[cpu]);
> + dev_pm_opp_put_supported_hw(drv->opp_tables[cpu]);
> }
> - kfree(opp_tables);
> + kfree(drv->opp_tables);
> +free_drv:
> + kfree(drv);
>
> return ret;
> }
>
> static int qcom_cpufreq_remove(struct platform_device *pdev)
> {
> - struct opp_table **opp_tables = platform_get_drvdata(pdev);
> + struct qcom_cpufreq_drv *drv = platform_get_drvdata(pdev);
> unsigned int cpu;
>
> platform_device_unregister(cpufreq_dt_pdev);
>
> for_each_possible_cpu(cpu)
> - dev_pm_opp_put_supported_hw(opp_tables[cpu]);
> + if (drv->opp_tables[cpu])
> + dev_pm_opp_put_supported_hw(drv->opp_tables[cpu]);
>
> - kfree(opp_tables);
> + kfree(drv->opp_tables);
> + kfree(drv);
>
> return 0;
> }
> @@ -220,10 +257,8 @@ static struct platform_driver qcom_cpufreq_driver = {
> };
>
> static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
> - { .compatible = "qcom,apq8096",
> - .data = qcom_cpufreq_kryo_name_version },
> - { .compatible = "qcom,msm8996",
> - .data = qcom_cpufreq_kryo_name_version },
> + { .compatible = "qcom,apq8096", .data = &match_data_kryo },
> + { .compatible = "qcom,msm8996", .data = &match_data_kryo },
> {},
> };
>
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH 03/13] dt-bindings: cpufreq: qcom-nvmem: Make speedbin related properties optional
From: Ilia Lin @ 2019-07-08 6:28 UTC (permalink / raw)
To: Niklas Cassel
Cc: Andy Gross, Ilia Lin, Viresh Kumar, Nishanth Menon, Stephen Boyd,
linux-arm-msm, jorge.ramirez-ortiz, bjorn.andersson, ulf.hansson,
Rob Herring, Mark Rutland, linux-pm, devicetree, linux-kernel
In-Reply-To: <20190705095726.21433-4-niklas.cassel@linaro.org>
Reviewed-by: Ilia Lin <ilia.lin@kernel.org>
On Fri, Jul 5, 2019 at 12:58 PM Niklas Cassel <niklas.cassel@linaro.org> wrote:
>
> Not all Qualcomm platforms need to care about the speedbin efuse,
> nor the value blown into the speedbin efuse.
> Therefore, make the nvmem-cells and opp-supported-hw properties
> optional.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
> Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> index 198441e80ba8..c5ea8b90e35d 100644
> --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> @@ -20,6 +20,10 @@ In 'cpus' nodes:
> In 'operating-points-v2' table:
> - compatible: Should be
> - 'operating-points-v2-kryo-cpu' for apq8096 and msm8996.
> +
> +Optional properties:
> +--------------------
> +In 'operating-points-v2' table:
> - nvmem-cells: A phandle pointing to a nvmem-cells node representing the
> efuse registers that has information about the
> speedbin that is used to select the right frequency/voltage
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH 01/13] dt-bindings: cpufreq: Re-organise kryo cpufreq to use it for other nvmem based qcom socs
From: Ilia Lin @ 2019-07-08 6:28 UTC (permalink / raw)
To: Niklas Cassel
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Ilia Lin, Andy Gross,
linux-arm-msm, jorge.ramirez-ortiz, bjorn.andersson, ulf.hansson,
Sricharan R, Rob Herring, Mark Rutland, linux-pm, devicetree,
linux-kernel
In-Reply-To: <20190705095726.21433-2-niklas.cassel@linaro.org>
Reviewed-by: Ilia Lin <ilia.lin@kernel.org>
On Fri, Jul 5, 2019 at 12:57 PM Niklas Cassel <niklas.cassel@linaro.org> wrote:
>
> From: Sricharan R <sricharan@codeaurora.org>
>
> The kryo cpufreq driver reads the nvmem cell and uses that data to
> populate the opps. There are other qcom cpufreq socs like krait which
> does similar thing. Except for the interpretation of the read data,
> rest of the driver is same for both the cases. So pull the common things
> out for reuse.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> [niklas.cassel@linaro.org: split dt-binding into a separate patch and
> do not rename the compatible string.]
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
> Changes since RFC:
> -Made DT bindings a separate patch.
> -Keep the original compatible string, since renaming it breaks DT
> backwards compatibility.
>
> .../opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt} | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
> rename Documentation/devicetree/bindings/opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt} (98%)
>
> diff --git a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> similarity index 98%
> rename from Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> rename to Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> index c2127b96805a..198441e80ba8 100644
> --- a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> @@ -1,13 +1,13 @@
> -Qualcomm Technologies, Inc. KRYO CPUFreq and OPP bindings
> +Qualcomm Technologies, Inc. NVMEM CPUFreq and OPP bindings
> ===================================
>
> -In Certain Qualcomm Technologies, Inc. SoCs like apq8096 and msm8996
> -that have KRYO processors, the CPU ferequencies subset and voltage value
> -of each OPP varies based on the silicon variant in use.
> +In Certain Qualcomm Technologies, Inc. SoCs like apq8096 and msm8996,
> +the CPU frequencies subset and voltage value of each OPP varies based on
> +the silicon variant in use.
> Qualcomm Technologies, Inc. Process Voltage Scaling Tables
> defines the voltage and frequency value based on the msm-id in SMEM
> and speedbin blown in the efuse combination.
> -The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
> +The qcom-cpufreq-nvmem driver reads the msm-id and efuse value from the SoC
> to provide the OPP framework with required information (existing HW bitmap).
> This is used to determine the voltage and frequency value for each OPP of
> operating-points-v2 table when it is parsed by the OPP framework.
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH 02/13] cpufreq: qcom: Re-organise kryo cpufreq to use it for other nvmem based qcom socs
From: Ilia Lin @ 2019-07-08 6:27 UTC (permalink / raw)
To: Niklas Cassel
Cc: Rafael J. Wysocki, Viresh Kumar, Andy Gross, Ilia Lin,
linux-arm-msm, jorge.ramirez-ortiz, Stephen Boyd, vireshk,
bjorn.andersson, ulf.hansson, Sricharan R, linux-kernel, linux-pm
In-Reply-To: <20190705095726.21433-3-niklas.cassel@linaro.org>
Reviewed-by: Ilia Lin <ilia.lin@kernel.org>
On Fri, Jul 5, 2019 at 12:57 PM Niklas Cassel <niklas.cassel@linaro.org> wrote:
>
> From: Sricharan R <sricharan@codeaurora.org>
>
> The kryo cpufreq driver reads the nvmem cell and uses that data to
> populate the opps. There are other qcom cpufreq socs like krait which
> does similar thing. Except for the interpretation of the read data,
> rest of the driver is same for both the cases. So pull the common things
> out for reuse.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> [niklas.cassel@linaro.org: split dt-binding into a separate patch and
> do not rename the compatible string. Update MAINTAINERS file.]
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
> Changes since RFC:
> -Made DT bindings a separate patch.
> -Keep the original compatible string, since renaming it breaks DT
> backwards compatibility.
>
> MAINTAINERS | 4 +-
> drivers/cpufreq/Kconfig.arm | 4 +-
> drivers/cpufreq/Makefile | 2 +-
> ...om-cpufreq-kryo.c => qcom-cpufreq-nvmem.c} | 122 +++++++++++-------
> 4 files changed, 78 insertions(+), 54 deletions(-)
> rename drivers/cpufreq/{qcom-cpufreq-kryo.c => qcom-cpufreq-nvmem.c} (70%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 04a66ba93c26..38ab8374fa7a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13242,8 +13242,8 @@ QUALCOMM CPUFREQ DRIVER MSM8996/APQ8096
> M: Ilia Lin <ilia.lin@kernel.org>
> L: linux-pm@vger.kernel.org
> S: Maintained
> -F: Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> -F: drivers/cpufreq/qcom-cpufreq-kryo.c
> +F: Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> +F: drivers/cpufreq/qcom-cpufreq-nvmem.c
>
> QUALCOMM EMAC GIGABIT ETHERNET DRIVER
> M: Timur Tabi <timur@kernel.org>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 56c31a78c692..b1aa485a28dd 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -120,8 +120,8 @@ config ARM_OMAP2PLUS_CPUFREQ
> depends on ARCH_OMAP2PLUS
> default ARCH_OMAP2PLUS
>
> -config ARM_QCOM_CPUFREQ_KRYO
> - tristate "Qualcomm Kryo based CPUFreq"
> +config ARM_QCOM_CPUFREQ_NVMEM
> + tristate "Qualcomm nvmem based CPUFreq"
> depends on ARM64
> depends on QCOM_QFPROM
> depends on QCOM_SMEM
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 5a6c70d26c98..8572a918aa75 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -64,7 +64,7 @@ obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
> obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o
> obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
> obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o
> -obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o
> +obj-$(CONFIG_ARM_QCOM_CPUFREQ_NVMEM) += qcom-cpufreq-nvmem.o
> obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o
> obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o
> obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o
> diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> similarity index 70%
> rename from drivers/cpufreq/qcom-cpufreq-kryo.c
> rename to drivers/cpufreq/qcom-cpufreq-nvmem.c
> index dd64dcf89c74..fad6509eecb5 100644
> --- a/drivers/cpufreq/qcom-cpufreq-kryo.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -9,7 +9,7 @@
> * based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
> * defines the voltage and frequency value based on the msm-id in SMEM
> * and speedbin blown in the efuse combination.
> - * The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
> + * The qcom-cpufreq driver reads the msm-id and efuse value from the SoC
> * to provide the OPP framework with required information.
> * This is used to determine the voltage and frequency value for each OPP of
> * operating-points-v2 table when it is parsed by the OPP framework.
> @@ -22,6 +22,7 @@
> #include <linux/module.h>
> #include <linux/nvmem-consumer.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/pm_opp.h>
> #include <linux/slab.h>
> @@ -42,9 +43,9 @@ enum _msm8996_version {
> NUM_OF_MSM8996_VERSIONS,
> };
>
> -static struct platform_device *cpufreq_dt_pdev, *kryo_cpufreq_pdev;
> +static struct platform_device *cpufreq_dt_pdev, *cpufreq_pdev;
>
> -static enum _msm8996_version qcom_cpufreq_kryo_get_msm_id(void)
> +static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
> {
> size_t len;
> u32 *msm_id;
> @@ -73,28 +74,62 @@ static enum _msm8996_version qcom_cpufreq_kryo_get_msm_id(void)
> return version;
> }
>
> -static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> +static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
> + struct nvmem_cell *speedbin_nvmem,
> + u32 *versions)
> {
> - struct opp_table **opp_tables;
> + size_t len;
> + u8 *speedbin;
> enum _msm8996_version msm8996_version;
> +
> + msm8996_version = qcom_cpufreq_get_msm_id();
> + if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
> + dev_err(cpu_dev, "Not Snapdragon 820/821!");
> + return -ENODEV;
> + }
> +
> + speedbin = nvmem_cell_read(speedbin_nvmem, &len);
> + if (IS_ERR(speedbin))
> + return PTR_ERR(speedbin);
> +
> + switch (msm8996_version) {
> + case MSM8996_V3:
> + *versions = 1 << (unsigned int)(*speedbin);
> + break;
> + case MSM8996_SG:
> + *versions = 1 << ((unsigned int)(*speedbin) + 4);
> + break;
> + default:
> + BUG();
> + break;
> + }
> +
> + kfree(speedbin);
> + return 0;
> +}
> +
> +static int qcom_cpufreq_probe(struct platform_device *pdev)
> +{
> + struct opp_table **opp_tables;
> + int (*get_version)(struct device *cpu_dev,
> + struct nvmem_cell *speedbin_nvmem,
> + u32 *versions);
> struct nvmem_cell *speedbin_nvmem;
> struct device_node *np;
> struct device *cpu_dev;
> unsigned cpu;
> - u8 *speedbin;
> u32 versions;
> - size_t len;
> + const struct of_device_id *match;
> int ret;
>
> cpu_dev = get_cpu_device(0);
> if (!cpu_dev)
> return -ENODEV;
>
> - msm8996_version = qcom_cpufreq_kryo_get_msm_id();
> - if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
> - dev_err(cpu_dev, "Not Snapdragon 820/821!");
> + match = pdev->dev.platform_data;
> + get_version = match->data;
> + if (!get_version)
> return -ENODEV;
> - }
>
> np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> if (!np)
> @@ -115,23 +150,10 @@ static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> return PTR_ERR(speedbin_nvmem);
> }
>
> - speedbin = nvmem_cell_read(speedbin_nvmem, &len);
> + ret = get_version(cpu_dev, speedbin_nvmem, &versions);
> nvmem_cell_put(speedbin_nvmem);
> - if (IS_ERR(speedbin))
> - return PTR_ERR(speedbin);
> -
> - switch (msm8996_version) {
> - case MSM8996_V3:
> - versions = 1 << (unsigned int)(*speedbin);
> - break;
> - case MSM8996_SG:
> - versions = 1 << ((unsigned int)(*speedbin) + 4);
> - break;
> - default:
> - BUG();
> - break;
> - }
> - kfree(speedbin);
> + if (ret)
> + return ret;
>
> opp_tables = kcalloc(num_possible_cpus(), sizeof(*opp_tables), GFP_KERNEL);
> if (!opp_tables)
> @@ -174,7 +196,7 @@ static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> return ret;
> }
>
> -static int qcom_cpufreq_kryo_remove(struct platform_device *pdev)
> +static int qcom_cpufreq_remove(struct platform_device *pdev)
> {
> struct opp_table **opp_tables = platform_get_drvdata(pdev);
> unsigned int cpu;
> @@ -189,18 +211,20 @@ static int qcom_cpufreq_kryo_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static struct platform_driver qcom_cpufreq_kryo_driver = {
> - .probe = qcom_cpufreq_kryo_probe,
> - .remove = qcom_cpufreq_kryo_remove,
> +static struct platform_driver qcom_cpufreq_driver = {
> + .probe = qcom_cpufreq_probe,
> + .remove = qcom_cpufreq_remove,
> .driver = {
> - .name = "qcom-cpufreq-kryo",
> + .name = "qcom-cpufreq",
> },
> };
>
> -static const struct of_device_id qcom_cpufreq_kryo_match_list[] __initconst = {
> - { .compatible = "qcom,apq8096", },
> - { .compatible = "qcom,msm8996", },
> - {}
> +static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
> + { .compatible = "qcom,apq8096",
> + .data = qcom_cpufreq_kryo_name_version },
> + { .compatible = "qcom,msm8996",
> + .data = qcom_cpufreq_kryo_name_version },
> + {},
> };
>
> /*
> @@ -209,7 +233,7 @@ static const struct of_device_id qcom_cpufreq_kryo_match_list[] __initconst = {
> * which may be defered as well. The init here is only registering
> * the driver and the platform device.
> */
> -static int __init qcom_cpufreq_kryo_init(void)
> +static int __init qcom_cpufreq_init(void)
> {
> struct device_node *np = of_find_node_by_path("/");
> const struct of_device_id *match;
> @@ -218,32 +242,32 @@ static int __init qcom_cpufreq_kryo_init(void)
> if (!np)
> return -ENODEV;
>
> - match = of_match_node(qcom_cpufreq_kryo_match_list, np);
> + match = of_match_node(qcom_cpufreq_match_list, np);
> of_node_put(np);
> if (!match)
> return -ENODEV;
>
> - ret = platform_driver_register(&qcom_cpufreq_kryo_driver);
> + ret = platform_driver_register(&qcom_cpufreq_driver);
> if (unlikely(ret < 0))
> return ret;
>
> - kryo_cpufreq_pdev = platform_device_register_simple(
> - "qcom-cpufreq-kryo", -1, NULL, 0);
> - ret = PTR_ERR_OR_ZERO(kryo_cpufreq_pdev);
> + cpufreq_pdev = platform_device_register_data(NULL, "qcom-cpufreq",
> + -1, match, sizeof(*match));
> + ret = PTR_ERR_OR_ZERO(cpufreq_pdev);
> if (0 == ret)
> return 0;
>
> - platform_driver_unregister(&qcom_cpufreq_kryo_driver);
> + platform_driver_unregister(&qcom_cpufreq_driver);
> return ret;
> }
> -module_init(qcom_cpufreq_kryo_init);
> +module_init(qcom_cpufreq_init);
>
> -static void __exit qcom_cpufreq_kryo_exit(void)
> +static void __exit qcom_cpufreq_exit(void)
> {
> - platform_device_unregister(kryo_cpufreq_pdev);
> - platform_driver_unregister(&qcom_cpufreq_kryo_driver);
> + platform_device_unregister(cpufreq_pdev);
> + platform_driver_unregister(&qcom_cpufreq_driver);
> }
> -module_exit(qcom_cpufreq_kryo_exit);
> +module_exit(qcom_cpufreq_exit);
>
> -MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver");
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. CPUfreq driver");
> MODULE_LICENSE("GPL v2");
> --
> 2.21.0
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox