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
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@roeck-us.net>
---
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.2On Mon, Oct 17, 2022 at 3:09 PM Guenter Roeck <linux@roeck-us.net> 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@roeck-us.net>
> ---
> 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
>
On Wed, Nov 09, 2022 at 08:15:17PM +0100, Rafael J. Wysocki wrote:
> On Mon, Oct 17, 2022 at 3:09 PM Guenter Roeck <linux@roeck-us.net> 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"?
>
No, I did mean "locked", as in "must be called with thermal zone device
mutex acquired".
locked:
void __thermal_zone_device_update(struct thermal_zone_device *tz,
enum thermal_notify_event event)
{
...
}
unlocked:
void thermal_zone_device_update(struct thermal_zone_device *tz,
enum thermal_notify_event event)
{
mutex_lock(&tz->lock);
if (device_is_registered(&tz->device))
__thermal_zone_device_update(tz, event);
mutex_unlock(&tz->lock);
}
Should I phrase or explain it differently ?
Thanks,
Guenter
On Thu, Nov 10, 2022 at 1:25 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Wed, Nov 09, 2022 at 08:15:17PM +0100, Rafael J. Wysocki wrote:
> > On Mon, Oct 17, 2022 at 3:09 PM Guenter Roeck <linux@roeck-us.net> 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"?
> >
> No, I did mean "locked", as in "must be called with thermal zone device
> mutex acquired".
>
> locked:
>
> void __thermal_zone_device_update(struct thermal_zone_device *tz,
> enum thermal_notify_event event)
> {
> ...
> }
>
> unlocked:
>
> void thermal_zone_device_update(struct thermal_zone_device *tz,
> enum thermal_notify_event event)
> {
> mutex_lock(&tz->lock);
> if (device_is_registered(&tz->device))
> __thermal_zone_device_update(tz, event);
> mutex_unlock(&tz->lock);
> }
Thanks for the explanation.
> Should I phrase or explain it differently ?
I would rather say "bare" or something like that so it is all clear to
people like me, but it is your call.
On Thu, Nov 10, 2022 at 02:01:49PM +0100, Rafael J. Wysocki wrote:
> On Thu, Nov 10, 2022 at 1:25 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > On Wed, Nov 09, 2022 at 08:15:17PM +0100, Rafael J. Wysocki wrote:
> > > On Mon, Oct 17, 2022 at 3:09 PM Guenter Roeck <linux@roeck-us.net> 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"?
> > >
> > No, I did mean "locked", as in "must be called with thermal zone device
> > mutex acquired".
> >
> > locked:
> >
> > void __thermal_zone_device_update(struct thermal_zone_device *tz,
> > enum thermal_notify_event event)
> > {
> > ...
> > }
> >
> > unlocked:
> >
> > void thermal_zone_device_update(struct thermal_zone_device *tz,
> > enum thermal_notify_event event)
> > {
> > mutex_lock(&tz->lock);
> > if (device_is_registered(&tz->device))
> > __thermal_zone_device_update(tz, event);
> > mutex_unlock(&tz->lock);
> > }
>
> Thanks for the explanation.
>
> > Should I phrase or explain it differently ?
>
> I would rather say "bare" or something like that so it is all clear to
> people like me, but it is your call.
I updated the commit description to use "must be called with thermal
device mutex held". I kept 'locked' in the subject; I don't think using
'bare' there would add any clarity. Hope that is ok.
Thanks,
Guenter
On Thu, Nov 10, 2022 at 3:12 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Thu, Nov 10, 2022 at 02:01:49PM +0100, Rafael J. Wysocki wrote:
> > On Thu, Nov 10, 2022 at 1:25 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > >
> > > On Wed, Nov 09, 2022 at 08:15:17PM +0100, Rafael J. Wysocki wrote:
> > > > On Mon, Oct 17, 2022 at 3:09 PM Guenter Roeck <linux@roeck-us.net> 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"?
> > > >
> > > No, I did mean "locked", as in "must be called with thermal zone device
> > > mutex acquired".
> > >
> > > locked:
> > >
> > > void __thermal_zone_device_update(struct thermal_zone_device *tz,
> > > enum thermal_notify_event event)
> > > {
> > > ...
> > > }
> > >
> > > unlocked:
> > >
> > > void thermal_zone_device_update(struct thermal_zone_device *tz,
> > > enum thermal_notify_event event)
> > > {
> > > mutex_lock(&tz->lock);
> > > if (device_is_registered(&tz->device))
> > > __thermal_zone_device_update(tz, event);
> > > mutex_unlock(&tz->lock);
> > > }
> >
> > Thanks for the explanation.
> >
> > > Should I phrase or explain it differently ?
> >
> > I would rather say "bare" or something like that so it is all clear to
> > people like me, but it is your call.
>
> I updated the commit description to use "must be called with thermal
> device mutex held". I kept 'locked' in the subject; I don't think using
> 'bare' there would add any clarity. Hope that is ok.
It is.
© 2016 - 2026 Red Hat, Inc.