On 28/08/18 14:53, Patrick Bellasi wrote: [...]
> static inline int __setscheduler_uclamp(struct task_struct *p, > const struct sched_attr *attr) > { > - if (attr->sched_util_min > attr->sched_util_max) > - return -EINVAL; > - if (attr->sched_util_max > SCHED_CAPACITY_SCALE) > - return -EINVAL; > + int group_id[UCLAMP_CNT] = { UCLAMP_NOT_VALID }; > + int lower_bound, upper_bound; > + struct uclamp_se *uc_se; > + int result = 0; > > - p->uclamp[UCLAMP_MIN] = attr->sched_util_min; > - p->uclamp[UCLAMP_MAX] = attr->sched_util_max; > + mutex_lock(&uclamp_mutex); > > - return 0; > + /* Find a valid group_id for each required clamp value */ > + if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) { > + 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; > + } > + > + result = uclamp_group_find(UCLAMP_MIN, attr->sched_util_min); > + if (result == -ENOSPC) { > + pr_err(UCLAMP_ENOSPC_FMT, "MIN"); > + goto done; > + } > + group_id[UCLAMP_MIN] = result; > + } > + if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MAX) { > + lower_bound = (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) > + ? attr->sched_util_min > + : p->uclamp[UCLAMP_MIN].value; > + > + if (lower_bound == UCLAMP_NOT_VALID) > + lower_bound = 0; > + if (attr->sched_util_max < lower_bound || > + attr->sched_util_max > SCHED_CAPACITY_SCALE) { > + result = -EINVAL; > + goto done; > + } > + > + result = uclamp_group_find(UCLAMP_MAX, attr->sched_util_max); > + if (result == -ENOSPC) { > + pr_err(UCLAMP_ENOSPC_FMT, "MAX"); > + goto done; > + } > + group_id[UCLAMP_MAX] = result; > + } Don't you have to reset result to 0 here (it seems what follows cannot fail anymore)? Otherwise this function will return latest uclamp_group_find return value, which will be interpreted as error if not 0. > + > + /* Update each required clamp group */ > + if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) { > + uc_se = &p->uclamp[UCLAMP_MIN]; > + uclamp_group_get(UCLAMP_MIN, group_id[UCLAMP_MIN], > + uc_se, attr->sched_util_min); > + } > + if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MAX) { > + uc_se = &p->uclamp[UCLAMP_MAX]; > + uclamp_group_get(UCLAMP_MAX, group_id[UCLAMP_MAX], > + uc_se, attr->sched_util_max); > + } > + > +done: > + mutex_unlock(&uclamp_mutex); > + > + return result; > +} Best, - Juri