Re: [PATCH v3 12/14] sched/core: uclamp: add system default clamps

From: Dietmar Eggemann
Date: Mon Aug 20 2018 - 06:18:48 EST


On 08/06/2018 06:39 PM, Patrick Bellasi wrote:
Clamp values cannot be tuned at the root cgroup level. Moreover, because
of the delegation model requirements and how the parent clamps
propagation works, if we want to enable subgroups to set a non null
util.min, we need to be able to configure the root group util.min to the
allow the maximum utilization (SCHED_CAPACITY_SCALE = 1024).

Why 1024 (100)? Would any non 0 value work here?

[...]

@@ -1269,6 +1296,75 @@ static inline void uclamp_group_get(struct task_struct *p,
uclamp_group_put(clamp_id, prev_group_id);
}
+int sched_uclamp_handler(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp,
+ loff_t *ppos)
+{
+ int group_id[UCLAMP_CNT] = { UCLAMP_NOT_VALID };
+ struct uclamp_se *uc_se;
+ int old_min, old_max;
+ int result;
+
+ mutex_lock(&uclamp_mutex);
+
+ old_min = sysctl_sched_uclamp_util_min;
+ old_max = sysctl_sched_uclamp_util_max;
+
+ result = proc_dointvec(table, write, buffer, lenp, ppos);
+ if (result)
+ goto undo;
+ if (!write)
+ goto done;
+
+ if (sysctl_sched_uclamp_util_min > sysctl_sched_uclamp_util_max)
+ goto undo;
+ if (sysctl_sched_uclamp_util_max > 1024)
+ goto undo;
+
+ /* Find a valid group_id for each required clamp value */
+ if (old_min != sysctl_sched_uclamp_util_min) {
+ result = uclamp_group_find(UCLAMP_MIN, sysctl_sched_uclamp_util_min);
+ if (result == -ENOSPC) {
+ pr_err("Cannot allocate more than %d UTIL_MIN clamp groups\n",
+ CONFIG_UCLAMP_GROUPS_COUNT);
+ goto undo;
+ }
+ group_id[UCLAMP_MIN] = result;
+ }
+ if (old_max != sysctl_sched_uclamp_util_max) {
+ result = uclamp_group_find(UCLAMP_MAX, sysctl_sched_uclamp_util_max);
+ if (result == -ENOSPC) {
+ pr_err("Cannot allocate more than %d UTIL_MAX clamp groups\n",
+ CONFIG_UCLAMP_GROUPS_COUNT);
+ goto undo;
+ }
+ group_id[UCLAMP_MAX] = result;
+ }
+
+ /* Update each required clamp group */
+ if (old_min != sysctl_sched_uclamp_util_min) {
+ uc_se = &uclamp_default[UCLAMP_MIN];
+ uclamp_group_get(NULL, UCLAMP_MIN, group_id[UCLAMP_MIN],
+ uc_se, sysctl_sched_uclamp_util_min);
+ }
+ if (old_max != sysctl_sched_uclamp_util_max) {
+ uc_se = &uclamp_default[UCLAMP_MAX];
+ uclamp_group_get(NULL, UCLAMP_MAX, group_id[UCLAMP_MAX],
+ uc_se, sysctl_sched_uclamp_util_max);
+ }
+
+ if (result) {
+undo:
+ sysctl_sched_uclamp_util_min = old_min;
+ sysctl_sched_uclamp_util_max = old_max;
+ }

This looks strange! In case uclamp_group_find() returns free_group_id instead of -ENOSPC, the sysctl min/max values are reset?

I was under the assumption that I could specify:

sysctl_sched_uclamp_util_min = 40 (for boosting)
sysctl_sched_uclamp_util_max = 80 (for clamping)

with an empty cpu controller hierarchy and then those values become the .effective values of (a first level) task group?

[...]