Re: [PATCH v2] thermal: devfreq_cooling: use local ops instead of global ops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/13/22 16:06, Lukasz Luba wrote:


On 4/13/22 15:58, Rafael J. Wysocki wrote:
On Fri, Mar 25, 2022 at 10:02 AM Lukasz Luba <lukasz.luba@xxxxxxx> wrote:

Hi Kant,

On 3/25/22 07:30, Kant Fan wrote:
Fix access illegal address problem in following condition:
There are muti devfreq cooling devices in system, some of them has
em model but other does not, energy model ops such as state2power will
append to global devfreq_cooling_ops when the cooling device with
em model register. It makes the cooling device without em model
also use devfreq_cooling_ops after appending when register later by
of_devfreq_cooling_register_power() or of_devfreq_cooling_register().

IPA governor regards the cooling devices without em model as a power actor because they also have energy model ops, and will access illegal address
at dfc->em_pd when execute cdev->ops->get_requested_power,
cdev->ops->state2power or cdev->ops->power2state.

Fixes: 615510fe13bd2 ("thermal: devfreq_cooling: remove old power model and use EM")
Cc: stable@xxxxxxxxxxxxxxx # 5.13+
Signed-off-by: Kant Fan <kant@xxxxxxxxxxxxxxxxx>
---
   drivers/thermal/devfreq_cooling.c | 25 ++++++++++++++++++-------
   1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
index 4310cb342a9f..d38a80adec73 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -358,21 +358,28 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
       struct thermal_cooling_device *cdev;
       struct device *dev = df->dev.parent;
       struct devfreq_cooling_device *dfc;
+     struct thermal_cooling_device_ops *ops;
       char *name;
       int err, num_opps;

-     dfc = kzalloc(sizeof(*dfc), GFP_KERNEL);
-     if (!dfc)
+     ops = kmemdup(&devfreq_cooling_ops, sizeof(*ops), GFP_KERNEL);
+     if (!ops)
               return ERR_PTR(-ENOMEM);

+     dfc = kzalloc(sizeof(*dfc), GFP_KERNEL);
+     if (!dfc) {
+             err = -ENOMEM;
+             goto free_ops;
+     }
+
       dfc->devfreq = df;

       dfc->em_pd = em_pd_get(dev);
       if (dfc->em_pd) {
-             devfreq_cooling_ops.get_requested_power =
+             ops->get_requested_power =
                       devfreq_cooling_get_requested_power;
-             devfreq_cooling_ops.state2power = devfreq_cooling_state2power; -             devfreq_cooling_ops.power2state = devfreq_cooling_power2state;
+             ops->state2power = devfreq_cooling_state2power;
+             ops->power2state = devfreq_cooling_power2state;

               dfc->power_ops = dfc_power;

@@ -407,8 +414,7 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
       if (!name)
               goto remove_qos_req;

-     cdev = thermal_of_cooling_device_register(np, name, dfc,
-                                               &devfreq_cooling_ops);
+     cdev = thermal_of_cooling_device_register(np, name, dfc, ops);
       kfree(name);

       if (IS_ERR(cdev)) {
@@ -429,6 +435,8 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
       kfree(dfc->freq_table);
   free_dfc:
       kfree(dfc);
+free_ops:
+     kfree(ops);

       return ERR_PTR(err);
   }
@@ -510,11 +518,13 @@ EXPORT_SYMBOL_GPL(devfreq_cooling_em_register);
   void devfreq_cooling_unregister(struct thermal_cooling_device *cdev)
   {
       struct devfreq_cooling_device *dfc;
+     const struct thermal_cooling_device_ops *ops;
       struct device *dev;

       if (IS_ERR_OR_NULL(cdev))
               return;

+     ops = cdev->ops;
       dfc = cdev->devdata;
       dev = dfc->devfreq->dev.parent;

@@ -525,5 +535,6 @@ void devfreq_cooling_unregister(struct thermal_cooling_device *cdev)

       kfree(dfc->freq_table);
       kfree(dfc);
+     kfree(ops);
   }
   EXPORT_SYMBOL_GPL(devfreq_cooling_unregister);


Thank you for updating it, LGTM

Reviewed-by: Lukasz Luba <lukasz.luba@xxxxxxx>

Applied as 5.19 material.

Lukasz, this had a conflict with your EM series, please double check
if my resolution in the bleeding-edge branch is correct.

OK, I'll let you know after I fetch and build that branch.

I've read the code and confirm you've do this correctly.
I've also built that branch with ENERGY_MODEL and DEVFREQ_COOLING
configs set - no issues observed.
Later this week I would use it for some other development
so I will test it as well.

Thank you for solving this!

Regards,
Lukasz



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux