| 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: <2369752.l80tmojpSQ@vostro.rjw.lan>
Date: Thu, 25 Oct 2012 00:01:28 +0200
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Nishanth Menon <nm@...com>
Cc: linux-pm <linux-pm@...r.kernel.org>,
MyungJoo Ham <myungjoo.ham@...sung.com>,
Kyungmin Park <kyungmin.park@...sung.com>,
Kevin Hilman <khilman@...com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] PM / OPP: predictable fail results for opp_find* functions
On Monday 22 of October 2012 08:33:19 Nishanth Menon wrote:
> Currently the opp_find* functions return -ENODEV when:
> a) it cant find a device (e.g. request for an OPP search on device
> which was not registered)
> b) When it cant find a match for the search strategy used
>
> This makes life a little in-efficient for users such as devfreq
> to make reasonable judgement before switching search strategies.
>
> So, standardize the return results as following:
> -EINVAL for bad pointer parameters
> -ENODEV when device cannot be found
> -ERANGE when search fails
>
> This has the following benefit for devfreq implementation:
> The search fails when an unregistered device pointer is provided.
> This is a trigger to change the search direction and search for
> a better fit, however, if we cannot differentiate between a valid
> search range failure Vs an unregistered device, second search goes
> through the same fail return condition. This can be avoided by
> appropriate handling of error return code.
>
> With this change, we also fix devfreq for the improved search
> strategy with updated error code.
>
> Cc: MyungJoo Ham <myungjoo.ham@...sung.com>
> Cc: Kyungmin Park <kyungmin.park@...sung.com>
> Cc: "Rafael J. Wysocki" <rjw@...k.pl>
> Cc: Kevin Hilman <khilman@...com>
> Cc: linux-pm@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
>
> Reviewed-by: Kevin Hilman <khilman@...com>
> Acked-by: MyungJoo Ham <myungjoo.ham@...sung.com>
> Signed-off-by: Nishanth Menon <nm@...com>
Applied to linux-pm.git/linux-next as v3.8 material.
Thanks,
Rafael
> ---
> V2:
> * commit message change - updated frome example code, to verbose description
> * picked up Acked-by and Reviewed-by tags from V1.
> * rebased to v3.7-rc2
> V1: https://patchwork.kernel.org/patch/1582111/
>
> drivers/base/power/opp.c | 27 +++++++++++++++++++--------
> drivers/devfreq/devfreq.c | 4 ++--
> 2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index 4730a58..99d0573 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -234,7 +234,10 @@ EXPORT_SYMBOL(opp_get_opp_count);
> *
> * Searches for exact match in the opp list and returns pointer to the matching
> * opp if found, else returns ERR_PTR in case of error and should be handled
> - * using IS_ERR.
> + * using IS_ERR. Error return values can be:
> + * EINVAL: for bad pointer
> + * ERANGE: no match found for search
> + * ENODEV: if device not found in list of registered devices
> *
> * Note: available is a modifier for the search. if available=true, then the
> * match is for exact matching frequency and is available in the stored OPP
> @@ -253,7 +256,7 @@ struct opp *opp_find_freq_exact(struct device *dev, unsigned long freq,
> bool available)
> {
> struct device_opp *dev_opp;
> - struct opp *temp_opp, *opp = ERR_PTR(-ENODEV);
> + struct opp *temp_opp, *opp = ERR_PTR(-ERANGE);
>
> dev_opp = find_device_opp(dev);
> if (IS_ERR(dev_opp)) {
> @@ -283,7 +286,11 @@ EXPORT_SYMBOL(opp_find_freq_exact);
> * for a device.
> *
> * Returns matching *opp and refreshes *freq accordingly, else returns
> - * ERR_PTR in case of error and should be handled using IS_ERR.
> + * ERR_PTR in case of error and should be handled using IS_ERR. Error return
> + * values can be:
> + * EINVAL: for bad pointer
> + * ERANGE: no match found for search
> + * ENODEV: if device not found in list of registered devices
> *
> * Locking: This function must be called under rcu_read_lock(). opp is a rcu
> * protected pointer. The reason for the same is that the opp pointer which is
> @@ -294,7 +301,7 @@ EXPORT_SYMBOL(opp_find_freq_exact);
> struct opp *opp_find_freq_ceil(struct device *dev, unsigned long *freq)
> {
> struct device_opp *dev_opp;
> - struct opp *temp_opp, *opp = ERR_PTR(-ENODEV);
> + struct opp *temp_opp, *opp = ERR_PTR(-ERANGE);
>
> if (!dev || !freq) {
> dev_err(dev, "%s: Invalid argument freq=%p\n", __func__, freq);
> @@ -303,7 +310,7 @@ struct opp *opp_find_freq_ceil(struct device *dev, unsigned long *freq)
>
> dev_opp = find_device_opp(dev);
> if (IS_ERR(dev_opp))
> - return opp;
> + return ERR_CAST(dev_opp);
>
> list_for_each_entry_rcu(temp_opp, &dev_opp->opp_list, node) {
> if (temp_opp->available && temp_opp->rate >= *freq) {
> @@ -326,7 +333,11 @@ EXPORT_SYMBOL(opp_find_freq_ceil);
> * for a device.
> *
> * Returns matching *opp and refreshes *freq accordingly, else returns
> - * ERR_PTR in case of error and should be handled using IS_ERR.
> + * ERR_PTR in case of error and should be handled using IS_ERR. Error return
> + * values can be:
> + * EINVAL: for bad pointer
> + * ERANGE: no match found for search
> + * ENODEV: if device not found in list of registered devices
> *
> * Locking: This function must be called under rcu_read_lock(). opp is a rcu
> * protected pointer. The reason for the same is that the opp pointer which is
> @@ -337,7 +348,7 @@ EXPORT_SYMBOL(opp_find_freq_ceil);
> struct opp *opp_find_freq_floor(struct device *dev, unsigned long *freq)
> {
> struct device_opp *dev_opp;
> - struct opp *temp_opp, *opp = ERR_PTR(-ENODEV);
> + struct opp *temp_opp, *opp = ERR_PTR(-ERANGE);
>
> if (!dev || !freq) {
> dev_err(dev, "%s: Invalid argument freq=%p\n", __func__, freq);
> @@ -346,7 +357,7 @@ struct opp *opp_find_freq_floor(struct device *dev, unsigned long *freq)
>
> dev_opp = find_device_opp(dev);
> if (IS_ERR(dev_opp))
> - return opp;
> + return ERR_CAST(dev_opp);
>
> list_for_each_entry_rcu(temp_opp, &dev_opp->opp_list, node) {
> if (temp_opp->available) {
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 0920d75..bee1c29 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -656,14 +656,14 @@ struct opp *devfreq_recommended_opp(struct device *dev, unsigned long *freq,
> opp = opp_find_freq_floor(dev, freq);
>
> /* If not available, use the closest opp */
> - if (opp == ERR_PTR(-ENODEV))
> + if (opp == ERR_PTR(-ERANGE))
> opp = opp_find_freq_ceil(dev, freq);
> } else {
> /* The freq is an lower bound. opp should be higher */
> opp = opp_find_freq_ceil(dev, freq);
>
> /* If not available, use the closest opp */
> - if (opp == ERR_PTR(-ENODEV))
> + if (opp == ERR_PTR(-ERANGE))
> opp = opp_find_freq_floor(dev, freq);
> }
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists