On 13-Mar 15:09, Peter Zijlstra wrote:

On Fri, Feb 08, 2019 at 10:05:40AM +0000, Patrick Bellasi wrote:

quoted

+static inline unsigned int uclamp_none(int clamp_id)
+{
+	if (clamp_id == UCLAMP_MIN)
+		return 0;
+	return SCHED_CAPACITY_SCALE;
+}
+
+static inline void uclamp_rq_update(struct rq *rq, unsigned int clamp_id)
+{
+	struct uclamp_bucket *bucket = rq->uclamp[clamp_id].bucket;
+	unsigned int max_value = uclamp_none(clamp_id);

That's 1024 for uclamp_max

quoted

+	unsigned int bucket_id;
+
+	/*
+	 * Both min and max clamps are MAX aggregated, thus the topmost
+	 * bucket with some tasks defines the rq's clamp value.
+	 */
+	bucket_id = UCLAMP_BUCKETS;
+	do {
+		--bucket_id;
+		if (!rq->uclamp[clamp_id].bucket[bucket_id].tasks)
+			continue;
+		max_value = bucket[bucket_id].value;

but this will then _lower_ it. That's not a MAX aggregate.

For uclamp_max we want max_value=1024 when there are no active tasks,
which means: no max clamp enforced on CFS/RT "idle" cpus.

If instead there are active RT/CFS tasks then we want the clamp value
of the max group, which means: MAX aggregate active clamps.

That's what the code above does and the comment says.

quoted

+		break;
+	} while (bucket_id);
+
+	WRITE_ONCE(rq->uclamp[clamp_id].value, max_value);
+}

-- 
#include <best/regards.h>

Patrick Bellasi