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
