2022-07-15 04:22:38

by Shivnandan Kumar

Subject: [PATCH v3]PM: QoS: Add check to make sure CPU freq is non-negative

CPU frequency should never be negative.
If some client driver calls freq_qos_update_request with
negative value which will be very high in absolute terms,
then qos driver set max CPU freq at fmax as it consider
it's absolute value but it will add plist node with negative
priority. plist node has priority from INT_MIN (highest)
to INT_MAX(lowest). Once priority is set as negative,
another client will not be able to reduce CPU frequency.
Adding check to make sure CPU freq is non-negative will
fix this problem.

Signed-off-by: Shivnandan Kumar <[email protected]>
---
v2->v3
-changed commit text
v1->v2
-addressed comments from Rafael
-changed commit text accordingly
kernel/power/qos.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index ec7e1e85923e..27e6596f287a 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -531,7 +531,7 @@ int freq_qos_add_request(struct freq_constraints *qos,
{
int ret;

- if (IS_ERR_OR_NULL(qos) || !req)
+ if (IS_ERR_OR_NULL(qos) || !req || value < FREQ_QOS_MIN_DEFAULT_VALUE)
return -EINVAL;

if (WARN(freq_qos_request_active(req),
@@ -563,7 +563,7 @@ EXPORT_SYMBOL_GPL(freq_qos_add_request);
*/
int freq_qos_update_request(struct freq_qos_request *req, s32 new_value)
{
- if (!req)
+ if (!req || new_value < FREQ_QOS_MIN_DEFAULT_VALUE)
return -EINVAL;

if (WARN(!freq_qos_request_active(req),
--
2.25.1

2022-07-15 12:00:35

by Rafael J. Wysocki

Subject: Re: [PATCH v3]PM: QoS: Add check to make sure CPU freq is non-negative

On Fri, Jul 15, 2022 at 6:10 AM Shivnandan Kumar
<[email protected]> wrote:
>
> CPU frequency should never be negative.
> If some client driver calls freq_qos_update_request with
> negative value which will be very high in absolute terms,
> then qos driver set max CPU freq at fmax as it consider
> it's absolute value but it will add plist node with negative
> priority. plist node has priority from INT_MIN (highest)
> to INT_MAX(lowest). Once priority is set as negative,
> another client will not be able to reduce CPU frequency.
> Adding check to make sure CPU freq is non-negative will
> fix this problem.
>
> Signed-off-by: Shivnandan Kumar <[email protected]>
> ---
> v2->v3
> -changed commit text
> v1->v2
> -addressed comments from Rafael
> -changed commit text accordingly
> kernel/power/qos.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index ec7e1e85923e..27e6596f287a 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -531,7 +531,7 @@ int freq_qos_add_request(struct freq_constraints *qos,
> {
> int ret;
>
> - if (IS_ERR_OR_NULL(qos) || !req)
> + if (IS_ERR_OR_NULL(qos) || !req || value < FREQ_QOS_MIN_DEFAULT_VALUE)

Why not just put 0 in there directly instead of FREQ_QOS_MIN_DEFAULT_VALUE?

As is, it is somewhat confusing (and same below).

> return -EINVAL;
>
> if (WARN(freq_qos_request_active(req),
> @@ -563,7 +563,7 @@ EXPORT_SYMBOL_GPL(freq_qos_add_request);
> */
> int freq_qos_update_request(struct freq_qos_request *req, s32 new_value)
> {
> - if (!req)
> + if (!req || new_value < FREQ_QOS_MIN_DEFAULT_VALUE)
> return -EINVAL;
>
> if (WARN(!freq_qos_request_active(req),
> --
> 2.25.1
>