Messages in this thread | | | Subject | Re: [Patch v3 7/7] sched: thermal: Enable tuning of decay period | From | Thara Gopinath <> | Date | Tue, 22 Oct 2019 16:36:13 -0400 |
| |
On 10/22/2019 05:03 AM, Quentin Perret wrote: > Hi Thara, > > On Monday 21 Oct 2019 at 17:03:56 (-0400), Thara Gopinath wrote: >> On 10/15/2019 06:14 AM, Quentin Perret wrote: >>> Hi Thara, >>> >>> On Sunday 13 Oct 2019 at 20:58:25 (-0400), Thara Gopinath wrote: >>>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >>>> index 00fcea2..5056c08 100644 >>>> --- a/kernel/sysctl.c >>>> +++ b/kernel/sysctl.c >>>> @@ -376,6 +376,13 @@ static struct ctl_table kern_table[] = { >>>> .mode = 0644, >>>> .proc_handler = proc_dointvec, >>>> }, >>>> + { >>>> + .procname = "sched_thermal_decay_coeff", >>>> + .data = &sysctl_sched_thermal_decay_coeff, >>>> + .maxlen = sizeof(unsigned int), >>>> + .mode = 0644, >>>> + .proc_handler = proc_dointvec, >>> >>> Perhaps change this for 'sched_proc_update_handler' with min and max >>> values ? Otherwise userspace is allowed to write nonsensical values >>> here. And since sysctl_sched_thermal_decay_coeff is used to shift, this >>> can lead to an undefined behaviour. >> Will do >>> >>> Also, could we take this sysctl out of SCHED_DEBUG ? I expect this to be >>> used/tuned on production devices where SCHED_DEBUG should theoretically >>> be off. >> >> I will take it out of SCHED_DEBUG. I am wondering if this should be >> a runtime control at all. Because this is a shift this changes the >> accumulating window for the thermal pressure signal. A runtime change >> will not guarantee a clean start of the window. May be I should make >> this a config option. > > I'd personally prefer if it wan't a Kconfig option. We'd like to make > Android devices (which are going to use this) work with a Generic Kernel > Image, which means there will be a single config for everyone. But I > expect this knob to be tuned to different values depending on the SoC. > > If you really don't want a sysctl, perhaps a cmdline option could work ? yes . I will. I have sent across v4 with these and other review comments fixed. > > Thanks, > Quentin >
-- Warm Regards Thara
| |