2023-03-30 13:45:32

by David Arcari

[permalink] [raw]
Subject: [PATCH] thermal: intel: powerclamp: Fix cpumask and max_idle module parameters

When cpumask is specified as a module parameter the value is
overwritten by the module init routine. This can easily be fixed
by checking to see if the mask has already been allocated in the
init routine.

When max_idle is specified as a module parameter a panic will occur.
The problem is that the idle_injection_cpu_mask is not allocated until
the module init routine executes. This can easily be fixed by allocating
the cpumask if it's not already allocated.

Fixes: ebf519710218 ("thermal: intel: powerclamp: Add two module parameters")

Signed-off-by: David Arcari <[email protected]>

Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Amit Kucheria <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Srinivas Pandruvada <[email protected]>
Cc: David Arcari <[email protected]>
Cc: Chen Yu <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
drivers/thermal/intel/intel_powerclamp.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/intel/intel_powerclamp.c b/drivers/thermal/intel/intel_powerclamp.c
index c7ba5680cd48..91fc7e239497 100644
--- a/drivers/thermal/intel/intel_powerclamp.c
+++ b/drivers/thermal/intel/intel_powerclamp.c
@@ -235,6 +235,12 @@ static int max_idle_set(const char *arg, const struct kernel_param *kp)
goto skip_limit_set;
}

+ if (!cpumask_available(idle_injection_cpu_mask)) {
+ ret = allocate_copy_idle_injection_mask(cpu_present_mask);
+ if (ret)
+ goto skip_limit_set;
+ }
+
if (check_invalid(idle_injection_cpu_mask, new_max_idle)) {
ret = -EINVAL;
goto skip_limit_set;
@@ -791,7 +797,8 @@ static int __init powerclamp_init(void)
return retval;

mutex_lock(&powerclamp_lock);
- retval = allocate_copy_idle_injection_mask(cpu_present_mask);
+ if (!cpumask_available(idle_injection_cpu_mask))
+ retval = allocate_copy_idle_injection_mask(cpu_present_mask);
mutex_unlock(&powerclamp_lock);

if (retval)
--
2.27.0


2023-03-30 17:52:04

by srinivas pandruvada

[permalink] [raw]
Subject: Re: [PATCH] thermal: intel: powerclamp: Fix cpumask and max_idle module parameters

On Thu, 2023-03-30 at 09:42 -0400, David Arcari wrote:
Reviewed-by: Srinivas Pandruvada <>> When cpumask is specified as a module parameter the value is
> overwritten by the module init routine.  This can easily be fixed
> by checking to see if the mask has already been allocated in the
> init routine.
>
> When max_idle is specified as a module parameter a panic will occur.
> The problem is that the idle_injection_cpu_mask is not allocated
> until
> the module init routine executes. This can easily be fixed by
> allocating
> the cpumask if it's not already allocated.
>
> Fixes: ebf519710218 ("thermal: intel: powerclamp: Add two module
> parameters")
>
> Signed-off-by: David Arcari <[email protected]>
Reviewed-by: Srinivas Pandruvada<[email protected]>

>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: Amit Kucheria <[email protected]>
> Cc: Zhang Rui <[email protected]>
> Cc: Srinivas Pandruvada <[email protected]>
> Cc: David Arcari <[email protected]>
> Cc: Chen Yu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> ---
>  drivers/thermal/intel/intel_powerclamp.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/intel/intel_powerclamp.c
> b/drivers/thermal/intel/intel_powerclamp.c
> index c7ba5680cd48..91fc7e239497 100644
> --- a/drivers/thermal/intel/intel_powerclamp.c
> +++ b/drivers/thermal/intel/intel_powerclamp.c
> @@ -235,6 +235,12 @@ static int max_idle_set(const char *arg, const
> struct kernel_param *kp)
>                 goto skip_limit_set;
>         }
>  
> +       if (!cpumask_available(idle_injection_cpu_mask)) {
> +               ret =
> allocate_copy_idle_injection_mask(cpu_present_mask);
> +               if (ret)
> +                       goto skip_limit_set;
> +       }
> +
>         if (check_invalid(idle_injection_cpu_mask, new_max_idle)) {
>                 ret = -EINVAL;
>                 goto skip_limit_set;
> @@ -791,7 +797,8 @@ static int __init powerclamp_init(void)
>                 return retval;
>  
>         mutex_lock(&powerclamp_lock);
> -       retval = allocate_copy_idle_injection_mask(cpu_present_mask);
> +       if (!cpumask_available(idle_injection_cpu_mask))
> +               retval =
> allocate_copy_idle_injection_mask(cpu_present_mask);
>         mutex_unlock(&powerclamp_lock);
>  
>         if (retval)

2023-03-30 18:07:53

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] thermal: intel: powerclamp: Fix cpumask and max_idle module parameters

On Thu, Mar 30, 2023 at 7:36 PM srinivas pandruvada
<[email protected]> wrote:
>
> On Thu, 2023-03-30 at 09:42 -0400, David Arcari wrote:
> Reviewed-by: Srinivas Pandruvada <>> When cpumask is specified as a module parameter the value is
> > overwritten by the module init routine. This can easily be fixed
> > by checking to see if the mask has already been allocated in the
> > init routine.
> >
> > When max_idle is specified as a module parameter a panic will occur.
> > The problem is that the idle_injection_cpu_mask is not allocated
> > until
> > the module init routine executes. This can easily be fixed by
> > allocating
> > the cpumask if it's not already allocated.
> >
> > Fixes: ebf519710218 ("thermal: intel: powerclamp: Add two module
> > parameters")
> >
> > Signed-off-by: David Arcari <[email protected]>
> Reviewed-by: Srinivas Pandruvada<[email protected]>

Applied as 6.3-rc material, thanks!

> > Cc: "Rafael J. Wysocki" <[email protected]>
> > Cc: Daniel Lezcano <[email protected]>
> > Cc: Amit Kucheria <[email protected]>
> > Cc: Zhang Rui <[email protected]>
> > Cc: Srinivas Pandruvada <[email protected]>
> > Cc: David Arcari <[email protected]>
> > Cc: Chen Yu <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> >
> > ---
> > drivers/thermal/intel/intel_powerclamp.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/intel/intel_powerclamp.c
> > b/drivers/thermal/intel/intel_powerclamp.c
> > index c7ba5680cd48..91fc7e239497 100644
> > --- a/drivers/thermal/intel/intel_powerclamp.c
> > +++ b/drivers/thermal/intel/intel_powerclamp.c
> > @@ -235,6 +235,12 @@ static int max_idle_set(const char *arg, const
> > struct kernel_param *kp)
> > goto skip_limit_set;
> > }
> >
> > + if (!cpumask_available(idle_injection_cpu_mask)) {
> > + ret =
> > allocate_copy_idle_injection_mask(cpu_present_mask);
> > + if (ret)
> > + goto skip_limit_set;
> > + }
> > +
> > if (check_invalid(idle_injection_cpu_mask, new_max_idle)) {
> > ret = -EINVAL;
> > goto skip_limit_set;
> > @@ -791,7 +797,8 @@ static int __init powerclamp_init(void)
> > return retval;
> >
> > mutex_lock(&powerclamp_lock);
> > - retval = allocate_copy_idle_injection_mask(cpu_present_mask);
> > + if (!cpumask_available(idle_injection_cpu_mask))
> > + retval =
> > allocate_copy_idle_injection_mask(cpu_present_mask);
> > mutex_unlock(&powerclamp_lock);
> >
> > if (retval)
>