Re: [PATCH v1 1/6] thermal: sysfs: Trigger zone temperature updates on sysfs reads

[Rafael J. Wysocki]

On Thu, May 16, 2024 at 12:02 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>
> Hi Daniel,
>
> On Thu, May 16, 2024 at 11:46 AM Daniel Lezcano
> <daniel.lezcano@xxxxxxxxxx> wrote:
> >
> >
> > Hi Rafael,
> >
> > On 16/05/2024 11:04, Rafael J. Wysocki wrote:
> > > Hi Lukasz,
> > >
> > > On Mon, May 13, 2024 at 9:11 AM Lukasz Luba <lukasz.luba@xxxxxxx> wrote:
> > >>
> > >> Hi Rafael,
> > >>
> > >> On 5/10/24 15:13, Rafael J. Wysocki wrote:
> > >>> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > >>>
> > >>> Reading the zone temperature via sysfs causes the driver callback to
> > >>> be invoked, but it does not cause the thermal zone object to be updated.
> > >>>
> > >>> This is problematic if the zone temperature read via sysfs differs from
> > >>> the temperature value stored in the thermal zone object as it may cause
> > >>> the kernel and user space to act against each other in some cases.
> > >>>
> > >>> For this reason, make temp_show() trigger a zone temperature update if
> > >>> the temperature returned by thermal_zone_get_temp() is different from
> > >>> the temperature value stored in the thermal zone object.
> >
> >
> > The hwmon system is doing something similar and I'm not sure we want to
> > mimic the same behavior.
> >
> > Just to summarize:
> >
> > 1. There is a polling delay set
> >
> > This polling delay gives the sampling rate the thermal zone is
> > monitored. The temperature is updated at each 'delay' tick
> >
> > 2. There is no polling delay set
> >
> > The system relies on the interrupts to tell when a temperature reaches a
> > threshold.
>
> So this is a bit of a problem if the interrupts are not coming.
>
> At least from the debugfs perspective, there are "mitigation episodes"
> that last forever if the zone temperature happens to be above a trip
> at the system resume time, say, and is never updated afterward.
>
> > On the other side, if the governor is in-kernel, then we should not read
> > the temperature of the thermal zones because it is the job of the kernel
> > to do that.
> >
> > Actually we can assume the temperature information exported to the
> > userspace is a courtesy of the kernel when this one is managing the
> > thermal zone.
>
> It is not the case right now, though, as sysfs temperature reads
> effectively bypass the whole in-kernel thermal management.
>
> > If there is no governor associated to the thermal zone because there is
> > no cooling device associated to the defined trip points, then we can
> > assume it is up to the userspace to monitor the thermal zone.
>
> Well, in that case trips should not be taken into account, but they are now.

Actually, there is always a governor associated with a thermal zone
AFAICS.  It is set before binding any cooling devices to the thermal
zone.

However, there are thermal zones without any cooling devices bound to
them and they have the governor set to user_space, so it appears that
they want to receive trip crossing notifications, but then they don't
have a way to trigger zone temperature updates which is inconsistent.

IMO at least for these thermal zones temp_store() should trigger a
zone temperature update.