Thread (16 messages) 16 messages, 5 authors, 2019-09-03

Re: [PATCH v14 1/6] sched/core: uclamp: Extend CPU's cgroup controller

From: Patrick Bellasi <hidden>
Date: 2019-09-02 06:41:16
Also in: cgroups, linux-pm, lkml

On Fri, Aug 30, 2019 at 09:45:05 +0000, Peter Zijlstra wrote...
On Thu, Aug 22, 2019 at 02:28:06PM +0100, Patrick Bellasi wrote:
quoted
+#define _POW10(exp) ((unsigned int)1e##exp)
+#define POW10(exp) _POW10(exp)
What is this magic? You're forcing a float literal into an integer.
Surely that deserves a comment!
Yes, I'm introducing the two constants:
  UCLAMP_PERCENT_SHIFT,
  UCLAMP_PERCENT_SCALE
similar to what we have for CAPACITY. Moreover, I need both 100*100 (for
the scale) and 100 further down in the code for the: 

	percent = div_u64_rem(percent, POW10(UCLAMP_PERCENT_SHIFT), &rem);

used in cpu_uclamp_print().

That's why adding a compile time support to compute a 10^N is useful.

C provides the "1eN" literal, I just convert it to integer and to do
that at compile time I need a two level macros.

What if I add this comment just above the macro definitions:

/*
 * Integer 10^N with a given N exponent by casting to integer the literal "1eN"
 * C expression. Since there is no way to convert a macro argument (N) into a
 * character constant, use two levels of macros.
 */

is this clear enough?
quoted
+struct uclamp_request {
+#define UCLAMP_PERCENT_SHIFT	2
+#define UCLAMP_PERCENT_SCALE	(100 * POW10(UCLAMP_PERCENT_SHIFT))
+	s64 percent;
+	u64 util;
+	int ret;
+};
+
+static inline struct uclamp_request
+capacity_from_percent(char *buf)
+{
+	struct uclamp_request req = {
+		.percent = UCLAMP_PERCENT_SCALE,
+		.util = SCHED_CAPACITY_SCALE,
+		.ret = 0,
+	};
+
+	buf = strim(buf);
+	if (strncmp("max", buf, 4)) {
That is either a bug, and you meant to write: strncmp(buf, "max", 3),
or it is not, and then you could've written: strcmp(buf, "max")
I don't think it's a bug.

The usage of 4 is intentional, to force a '\0' check while using
strncmp(). Otherwise, strncmp(buf, "max", 3) would accept also strings
starting by "max", which we don't want.
But as written it doesn't make sense.
The code is safe but I agree that strcmp() does just the same and it
does not generate confusion. That's actually a pretty good example
on how it's not always better to use strncmp() instead of strcmp().

Cheers,
Patrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help