linux-kernel - Re: [RFC][PATCH 4/5] thermal: Add support for setting polling interval

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 May 2020 12:25:00 +0200
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        rui.zhang@...el.com, amit.kucheria@...durent.com
Cc:     linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [RFC][PATCH 4/5] thermal: Add support for setting polling
 interval

On 19/05/2020 01:46, Srinivas Pandruvada wrote:
> On Mon, 2020-05-18 at 18:51 +0200, Daniel Lezcano wrote:
>> On 04/05/2020 20:16, Srinivas Pandruvada wrote:
>>> Add new attribute in the thermal syfs for setting temperature
>>> sampling
>>> interval when CONFIG_THERMAL_USER_EVENT_INTERFACE is defined. The
>>> default
>>> value is 0, which means no polling.
>>>
>>> At this interval user space will get an event THERMAL_TEMP_SAMPLE
>>> with
>>> temperature sample. This reuses existing polling mecahnism when
>>> polling
>>> or passive delay is specified during zone registry. To avoid
>>> interference
>>> with passive and polling delay, this new polling attribute can't be
>>> used
>>> for those zones.
>>
>> The userspace can get the temperature whenever it wants via the
>> temperature file. The polling is designed for a specific hardware and
>> the slope of the temperature graphic.
>>
>> The userspace has the alternative of reading the temperature based on
>> its own timer or wait for (and stick to) the thermal framework
>> sampling
>> rate. Adding a notification in the update is enough IMO.
>>
> The problem with this approach is that the user can't change sampling
> interval. Those polling intervals are fixed during thermal-zone
> register. Is there any way to change those defaults from user space?

No, we can't but the userspace can decide when to read the temperature
(via sysfs or netlink) and thus decide its own sampling rate.

Otherwise, we are talking about an userspace governor, so the platform
is setup with the desired sampling rate + userspace governor.

> Kernel can start with some long polling interval and user space can
> change close to some trip.

Ok, let me rephrase it. This (big) comment encompass also patch 3/5.

I understood now the initial need of adding user trip points.

There are platforms where the interrupt mode does not exist so setting
an user trip point does not set the interrupt for the closer
temperature, hence we end up with a kernel sampling rate and in this
case adding a trip point + new user sampling rate is pointless as the
userspace can poll the temperature at its convenient rate.

If we summarize the different combinations we have:

1. monitoring : interrupt mode, mitigation : interrupt mode

There are no thermal zone update until an interrupt fires. The
mitigation is based on trip point crossed.

2. monitoring : interrupt mode, mitigation : polling

There are no thermal zone update until an interrupt fires. The
mitigation happens with a sampling rate specified with the polling rate.

3. monitoring : polling, mitigation : polling

The thermal zone is updated at the polling rate, the mitigation occurs
with an update at the second polling rate.

IIUC, the RFC proposes to add a new type of temperature threshold,
followed a new polling rate to update the userspace.

IMHO, it is not a good thing to delegate to the kernel what the
userspace can handle easily.

I suggest:

 - Not add another polling rate. If the thermal zone has a polling rate
or supports the interrupt mode, then the user trip point setup succeed
otherwise it fails and up to the userspace to read the temperature at
its convenient rate. (Note multiple process may want to get temperature,
so one should not set the rate of others).

 - Not add another temp threshold structure but add a new trip type
"user" and keep using the existing trip structures, so the notification
can happen in the handle_trip_point function. The sysfs only reflects
the setup via the "trip_point_x_hyst", "trip_point_0_temp",
"trip_point_x_type"

 - Do not use sysfs for setup but rely on the genetlink for one message
setup instead of multiple sysfs file writing. Adding a trip point will
be straighforward.

What do you think?

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog