Messages in this thread |  | | Subject | Re: [Patch v9 7/8] sched/fair: Enable tuning of decay period | From | Dietmar Eggemann <> | Date | Mon, 10 Feb 2020 12:59:50 +0100 |
| |
On 07/02/2020 23:42, Thara Gopinath wrote: > On 02/04/2020 03:39 AM, Dietmar Eggemann wrote: >> On 03/02/2020 16:55, Peter Zijlstra wrote: >>> On Mon, Feb 03, 2020 at 07:07:57AM -0500, Thara Gopinath wrote: >>>> On 01/28/2020 06:56 PM, Randy Dunlap wrote: >>>>> Hi, >>>>> >>>>> On 1/28/20 2:36 PM, Thara Gopinath wrote:
[...]
>> I do agree. IMHO, there are just two little things outstanding: >> >> (1) arch_scale_thermal_pressure() instead of >> arch_cpu_thermal_pressure() in v8 4/7 > > The "scale_" part was discussed in v6. Ionela had suggested that having > "scale" is not suited for this function because "thermal pressure" is > not exactly scaled but subtracted. I actually agree with that. > > https://lore.kernel.org/lkml/20191223175005.GA31446@arm.com/ > > Having said that if everyone feel the same about naming of this > function, I can change it one last time.
I'm still in favor for this. And Vincent seems to be OK as well:
https://lore.kernel.org/r/CAKfTPtBzoLnvAJ7sjPogMYS=WwBbdzWO07Kj=KDFVpO4=Su5ow@mail.gmail.com
The 'v6->v7' change note:
- Renamed arch_scale_thermal_capacity to arch_cpu_thermal_pressure as per review comments from Peter, Dietmar and Ionela.
is really not saying from which review comment the individual changes in the function name are coming from. And I don't see an answer to Ionela's email saying that her proposal will manifest in a particular part of this change.
>> (2) guarding of thermal pressure code in Arm's arch_topology driver w/ >> CONFIG_HAVE_SCHED_THERMAL_PRESSURE plus disabling it by default for >> Arm64. > It was enabled by default as per your suggestion in v9. > > The patch can be dropped. > > I don't understand the need to guard arch_topology with > CONFIG_HAVE_SCHED_THERMAL_PRESSURE. CONFIG_HAVE_SCHED_THERMAL_PRESSURE > is for scheduler to enable/disable averaging of thermal pressure. We > wanted to separate updating and retrieving of instantaneous thermal > pressure from scheduler. Guarding it with > CONFIG_HAVE_SCHED_THERMAL_PRESSURE is to me equivalent to putting back > this whole code in the scheduler framework. I am against it. I also do > not see other arch_ functions guarded similarly.
Cpu-invariant accounting can't be guarded with a kernel CONFIG switch. Frequency-invariant accounting could be with CONFIG_CPU_FREQ but this is enabled by default by Arm64 defconfig. Thermal pressure (accounting) (CONFIG_HAVE_SCHED_THERMAL_PRESSURE) is disabled by default so why should a per-cpu thermal_pressure be maintained on such a system (CONFIG_CPU_THERMAL=y by default)?
|  |