diff mbox series

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

Message ID 20220715122539.3978614-1-quic_kshivnan@quicinc.com
State Superseded
Headers show
Series [v4] PM: QoS: Add check to make sure CPU freq is non-negative | expand

Commit Message

Shivnandan Kumar July 15, 2022, 12:25 p.m. UTC
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 sets max CPU freq at fmax as it considers
	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 <quic_kshivnan@quicinc.com>
---
v3->v4
	-used 0 instead of FREQ_QOS_MIN_DEFAULT_VALUE
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(-)

Comments

Rafael J. Wysocki July 26, 2022, 6:49 p.m. UTC | #1
On Fri, Jul 15, 2022 at 2:27 PM Shivnandan Kumar
<quic_kshivnan@quicinc.com> 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 sets max CPU freq at fmax as it considers
>         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 <quic_kshivnan@quicinc.com>
> ---
> v3->v4
>         -used 0 instead of FREQ_QOS_MIN_DEFAULT_VALUE
> 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..af51ed6d45ef 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 < 0)
>                 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 < 0)
>                 return -EINVAL;
>
>         if (WARN(!freq_qos_request_active(req),
> --

Applied as 5.20 material, thanks!
diff mbox series

Patch

diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index ec7e1e85923e..af51ed6d45ef 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 < 0)
 		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 < 0)
 		return -EINVAL;
 
 	if (WARN(!freq_qos_request_active(req),