Messages in this thread | | | Date | Wed, 12 Sep 2018 18:42:09 +0100 | From | Patrick Bellasi <> | Subject | Re: [PATCH v4 02/16] sched/core: uclamp: map TASK's clamp values into CPU's clamp groups |
| |
On 12-Sep 18:24, Peter Zijlstra wrote: > On Tue, Aug 28, 2018 at 02:53:10PM +0100, Patrick Bellasi wrote: > > static inline int __setscheduler_uclamp(struct task_struct *p, > > const struct sched_attr *attr) > > But large for inline now.
Yes, Suren also already pointed that out... already gone in my v5 ;)
> > { > > + int group_id[UCLAMP_CNT] = { UCLAMP_NOT_VALID }; > > + int lower_bound, upper_bound; > > + struct uclamp_se *uc_se; > > + int result = 0; > > I think the thing would become much more readable if you set > lower/upper_bound right here.
Do you mean the bits I've ---8<---ed below ?
> > > > + mutex_lock(&uclamp_mutex); > > > > + /* Find a valid group_id for each required clamp value */ > > + if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) {
---8<--- > > + upper_bound = (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MAX) > > + ? attr->sched_util_max > > + : p->uclamp[UCLAMP_MAX].value; > > + > > + if (upper_bound == UCLAMP_NOT_VALID) > > + upper_bound = SCHED_CAPACITY_SCALE; > > + if (attr->sched_util_min > upper_bound) { > > + result = -EINVAL; > > + goto done; > > + } ---8<---
Actually it could also make sense to have them before the mutex ;)
> > + > > + result = uclamp_group_find(UCLAMP_MIN, attr->sched_util_min); > > + if (result == -ENOSPC) { > > + pr_err(UCLAMP_ENOSPC_FMT, "MIN"); > > AFAICT this is an unpriv part of the syscall; and you can spam the log > without limits. Not good.
Good point, will better check this.
[...]
Cheers, Patrick
-- #include <best/regards.h>
Patrick Bellasi
| |