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
| ||
|
Date: Mon, 26 Oct 2020 23:45:38 +0800 From: Yun Hsiang <hsiang023167@...il.com> To: Dietmar Eggemann <dietmar.eggemann@....com> Cc: peterz@...radead.org, linux-kernel@...r.kernel.org, qais.yousef@....com, patrick.bellasi@...bug.net Subject: Re: [PATCH v3 1/1] sched/uclamp: add SCHED_FLAG_UTIL_CLAMP_RESET flag to reset uclamp Hi Dietmar, On Mon, Oct 26, 2020 at 10:47:11AM +0100, Dietmar Eggemann wrote: > On 25/10/2020 08:36, Yun Hsiang wrote: > > If the user wants to stop controlling uclamp and let the task inherit > > the value from the group, we need a method to reset. > > > > Add SCHED_FLAG_UTIL_CLAMP_RESET flag to allow the user to reset uclamp via > > sched_setattr syscall. > > > > The policy is > > _CLAMP_RESET => reset both min and max > > _CLAMP_RESET | _CLAMP_MIN => reset min value > > _CLAMP_RESET | _CLAMP_MAX => reset max value > > _CLAMP_RESET | _CLAMP_MIN | _CLAMP_MAX => reset both min and max > > > > Signed-off-by: Yun Hsiang <hsiang023167@...il.com> > > [...] > > > @@ -1451,7 +1464,8 @@ static void __setscheduler_uclamp(struct task_struct *p, > > struct uclamp_se *uc_se = &p->uclamp_req[clamp_id]; > > > > /* Keep using defined clamps across class changes */ > > - if (uc_se->user_defined) > > + if (flags != SCHED_FLAG_UTIL_CLAMP_RESET && > > + uc_se->user_defined) > > continue; > > With: > > (1) _CLAMP_RESET => reset both min and max > (2) _CLAMP_RESET | _CLAMP_MIN => reset min value > (3) _CLAMP_RESET | _CLAMP_MAX => reset max value > (4) _CLAMP_RESET | _CLAMP_MIN | _CLAMP_MAX => reset both min and max > > If you reset an RT task with (1) you don't reset its uclamp.min value. > > __uclamp_update_util_min_rt_default(p) doesn't know about > SCHED_FLAG_UTIL_CLAMP_RESET. It only knows user_defined and will bail early. > Sorry I didn't notice __uclamp_update_util_min_rt_default will return directly if user_defined is set, I'll fix it. > [...] > > > - if (likely(!(attr->sched_flags & SCHED_FLAG_UTIL_CLAMP))) > > + if (likely(!flags || flags == SCHED_FLAG_UTIL_CLAMP_RESET)) > > return; > > > > - if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) { > > + if (flags & SCHED_FLAG_UTIL_CLAMP_MIN) { > > + if (reset) { > > + clamp_value = __default_uclamp_value(p, UCLAMP_MIN); > > + user_defined = false; > > + } else { > > + clamp_value = attr->sched_util_min; > > + user_defined = true; > > + } > > Why do you reset for (1) in the for_each_clamp_id(clamp_id) loop and for > (2)-(4) in the if (flags & SCHED_FLAG_UTIL_CLAMP_MXX) condition? > > You could reset (1)-(4) in the for_each_clamp_id(clamp_id) loop? In this > case you wouldn't need __default_uclamp_value(). Do you mean adding these code in for_each_clamp_id(clamp_id) loop? if (clamp_id == UCLAMP_MIN) { if (flags == SCHED_FLAG_UTIL_CLAMP_RESET || (reset && (flags || SCHED_FLAG_UTIL_CLAMP_MIN)) || !se->user_defined) { if (task_rt(p)) { clamp_value = sysctl_sched_uclamp_util_min_rt_default } else { clamp_value = uclamp_none(clamp_id); } } else continue; } /* similar code for UCLAMP_MAX */ ... uclamp_se_set(uc_se, clamp_value, false); It seems more clear. If you think this one is better, I'll use this method and send patch V4. > [...]
Powered by blists - more mailing lists