| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ea409e2f-f3ca-437f-d787-7ba793a2c226@samsung.com>
Date: Wed, 3 Feb 2021 19:11:19 +0900
From: Chanwoo Choi <cw00.choi@...sung.com>
To: Lukasz Luba <lukasz.luba@....com>, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org
Cc: vireshk@...nel.org, rafael@...nel.org, daniel.lezcano@...aro.org,
Dietmar.Eggemann@....com, amitk@...nel.org, rui.zhang@...el.com,
myungjoo.ham@...sung.com, kyungmin.park@...sung.com
Subject: Re: [RFC][PATCH 1/3] PM /devfreq: add user frequency limits into
devfreq struct
Hi Lukasz,
When accessing the max_freq and min_freq at devfreq-cooling.c,
even if can access 'user_max_freq' and 'lock' by using the 'devfreq' instance,
I think that the direct access of variables (lock/user_max_freq/user_min_freq)
of struct devfreq are not good.
Instead, how about using the 'DEVFREQ_TRANSITION_NOTIFIER'
notification with following changes of 'struct devfreq_freq'?
Also, need to add codes into devfreq_set_target() for initializing
'new_max_freq' and 'new_min_freq' before sending the DEVFREQ_POSTCHANGE
notification.
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 147a229056d2..d5726592d362 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -207,6 +207,8 @@ struct devfreq {
struct devfreq_freqs {
unsigned long old;
unsigned long new;
+ unsigned long new_max_freq;
+ unsigned long new_min_freq;
};
And I think that new 'user_min_freq'/'user_max_freq' are not necessary.
You can get the current max_freq/min_freq by using the following steps:
get_freq_range(devfreq, &min_freq, &max_freq);
dev_pm_opp_find_freq_floor(pdev, &min_freq);
dev_pm_opp_find_freq_floor(pdev, &max_freq);
So that you can get the 'max_freq/min_freq' and then
initialize the 'freqs.new_max_freq and freqs.new_min_freq'
with them as following:
in devfreq_set_target()
get_freq_range(devfreq, &min_freq, &max_freq);
dev_pm_opp_find_freq_floor(pdev, &min_freq);
dev_pm_opp_find_freq_floor(pdev, &max_freq);
freqs.new_max_freq = min_freq;
freqs.new_max_freq = max_freq;
devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE);
On 1/26/21 7:39 PM, Lukasz Luba wrote:
> The new fields inside devfreq struct allow to check the frequency limits
> set by the user via sysfs. These limits are important for thermal governor
> Intelligent Power Allocation (IPA) which needs to know the maximum allowed
> power consumption of the device.
>
> Signed-off-by: Lukasz Luba <lukasz.luba@....com>
> ---
> drivers/devfreq/devfreq.c | 41 +++++++++++++++++++++++++++++++++++----
> include/linux/devfreq.h | 4 ++++
> 2 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 94cc25fd68da..e985a76e5ff3 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -843,6 +843,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
> goto err_dev;
> }
>
> + devfreq->user_min_freq = devfreq->scaling_min_freq;
> + devfreq->user_max_freq = devfreq->scaling_max_freq;
> +
> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> atomic_set(&devfreq->suspend_count, 0);
>
> @@ -1513,6 +1516,8 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct devfreq *df = to_devfreq(dev);
> + struct device *pdev = df->dev.parent;
> + struct dev_pm_opp *opp;
> unsigned long value;
> int ret;
>
> @@ -1533,6 +1538,19 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
> if (ret < 0)
> return ret;
>
> + if (!value)
> + value = df->scaling_min_freq;
> +
> + opp = dev_pm_opp_find_freq_ceil(pdev, &value);
> + if (IS_ERR(opp))
> + return count;
> +
> + dev_pm_opp_put(opp);
> +
> + mutex_lock(&df->lock);
> + df->user_min_freq = value;
> + mutex_unlock(&df->lock);
> +
> return count;
> }
>
> @@ -1554,7 +1572,9 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct devfreq *df = to_devfreq(dev);
> - unsigned long value;
> + struct device *pdev = df->dev.parent;
> + unsigned long value, value_khz;
> + struct dev_pm_opp *opp;
> int ret;
>
> /*
> @@ -1579,14 +1599,27 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
> * A value of zero means "no limit".
> */
> if (value)
> - value = DIV_ROUND_UP(value, HZ_PER_KHZ);
> + value_khz = DIV_ROUND_UP(value, HZ_PER_KHZ);
> else
> - value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
> + value_khz = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
>
> - ret = dev_pm_qos_update_request(&df->user_max_freq_req, value);
> + ret = dev_pm_qos_update_request(&df->user_max_freq_req, value_khz);
> if (ret < 0)
> return ret;
>
> + if (!value)
> + value = df->scaling_max_freq;
> +
> + opp = dev_pm_opp_find_freq_floor(pdev, &value);
> + if (IS_ERR(opp))
> + return count;
> +
> + dev_pm_opp_put(opp);
> +
> + mutex_lock(&df->lock);
> + df->user_max_freq = value;
> + mutex_unlock(&df->lock);
> +
> return count;
> }
>
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index b6d3bae1c74d..147a229056d2 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -147,6 +147,8 @@ struct devfreq_stats {
> * touch this.
> * @user_min_freq_req: PM QoS minimum frequency request from user (via sysfs)
> * @user_max_freq_req: PM QoS maximum frequency request from user (via sysfs)
> + * @user_min_freq: User's minimum frequency
> + * @user_max_freq: User's maximum frequency
> * @scaling_min_freq: Limit minimum frequency requested by OPP interface
> * @scaling_max_freq: Limit maximum frequency requested by OPP interface
> * @stop_polling: devfreq polling status of a device.
> @@ -185,6 +187,8 @@ struct devfreq {
> struct dev_pm_qos_request user_max_freq_req;
> unsigned long scaling_min_freq;
> unsigned long scaling_max_freq;
> + unsigned long user_min_freq;
> + unsigned long user_max_freq;
> bool stop_polling;
>
> unsigned long suspend_freq;
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Powered by blists - more mailing lists