Re: [PATCH 5/9] thermal/core: Introduce locked version of thermal_zone_device_update

[Rafael J. Wysocki]

On Mon, Oct 17, 2022 at 3:09 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> In thermal_zone_device_set_mode(), the thermal zone mutex is released only
> to be reacquired in the subsequent call to thermal_zone_device_update().
>
> Introduce __thermal_zone_device_update() as locked version of

Did you mean "unlocked"?

> thermal_zone_device_update() and call it from
> thermal_zone_device_set_mode() without releasing the lock to avoid
> the extra release/acuire sequence.
>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
>  drivers/thermal/thermal_core.c | 57 ++++++++++++++++++----------------
>  1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 562ece8d16aa..9facd9c5b70f 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -403,6 +403,34 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz)
>                 pos->initialized = false;
>  }
>
> +static void __thermal_zone_device_update(struct thermal_zone_device *tz,
> +                                        enum thermal_notify_event event)
> +{
> +       int count;
> +
> +       if (atomic_read(&in_suspend))
> +               return;
> +
> +       if (WARN_ONCE(!tz->ops->get_temp,
> +                     "'%s' must not be called without 'get_temp' ops set\n",
> +                     __func__))
> +               return;
> +
> +       if (!thermal_zone_device_is_enabled(tz))
> +               return;
> +
> +       update_temperature(tz);
> +
> +       __thermal_zone_set_trips(tz);
> +
> +       tz->notify_event = event;
> +
> +       for (count = 0; count < tz->num_trips; count++)
> +               handle_thermal_trip(tz, count);
> +
> +       monitor_thermal_zone(tz);
> +}
> +
>  static int thermal_zone_device_set_mode(struct thermal_zone_device *tz,
>                                         enum thermal_device_mode mode)
>  {
> @@ -423,9 +451,9 @@ static int thermal_zone_device_set_mode(struct thermal_zone_device *tz,
>         if (!ret)
>                 tz->mode = mode;
>
> -       mutex_unlock(&tz->lock);
> +       __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>
> -       thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> +       mutex_unlock(&tz->lock);
>
>         if (mode == THERMAL_DEVICE_ENABLED)
>                 thermal_notify_tz_enable(tz->id);
> @@ -457,31 +485,8 @@ int thermal_zone_device_is_enabled(struct thermal_zone_device *tz)
>  void thermal_zone_device_update(struct thermal_zone_device *tz,
>                                 enum thermal_notify_event event)
>  {
> -       int count;
> -
> -       if (atomic_read(&in_suspend))
> -               return;
> -
> -       if (WARN_ONCE(!tz->ops->get_temp, "'%s' must not be called without "
> -                     "'get_temp' ops set\n", __func__))
> -               return;
> -
>         mutex_lock(&tz->lock);
> -
> -       if (!thermal_zone_device_is_enabled(tz))
> -               goto out;
> -
> -       update_temperature(tz);
> -
> -       __thermal_zone_set_trips(tz);
> -
> -       tz->notify_event = event;
> -
> -       for (count = 0; count < tz->num_trips; count++)
> -               handle_thermal_trip(tz, count);
> -
> -       monitor_thermal_zone(tz);
> -out:
> +       __thermal_zone_device_update(tz, event);
>         mutex_unlock(&tz->lock);
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_device_update);
> --
> 2.36.2
>