[PATCH 0/5] Add a new uclampset utility

[PATCH 0/5] Add a new uclampset utility

[Qais Yousef]

Since kernel v5.3 we have a new feature called utilization clamping that allows
influencing the utilization signals of a task, ultimately influencing the
performance of these tasks.

The series adds a new utility called uclampset that allows the user to
manipulate util clamp (or uclamp for short) for existing running processes or
when running a new command; in a similar spirit to how taskset and chrt
currently work.

Peter/Dietmar/Vincent/Patrick; reviewing the manpage (patch 3) to make sure it
explains this feature right would be much appreciated.

Thanks

--
Qais Yousef

Qais Yousef (5):
  Move sched_attr struct and syscall definitions into header file
  Add uclampset schedutil
  uclampset: Add man page
  uclampset: Plump into the build system
  uclampset: Plumb in bash-completion

 .gitignore                    |   1 +
 bash-completion/Makemodule.am |   3 +
 bash-completion/uclampset     |  39 ++++
 configure.ac                  |   9 +
 include/sched_attr.h          | 127 ++++++++++++
 schedutils/Makemodule.am      |   7 +
 schedutils/chrt.c             |  77 +------
 schedutils/uclampset.1        | 169 +++++++++++++++
 schedutils/uclampset.c        | 378 ++++++++++++++++++++++++++++++++++
 9 files changed, 734 insertions(+), 76 deletions(-)
 create mode 100644 bash-completion/uclampset
 create mode 100644 include/sched_attr.h
 create mode 100644 schedutils/uclampset.1
 create mode 100644 schedutils/uclampset.c

-- 
2.25.1

[PATCH 1/5] Move sched_attr struct and syscall definitions into header file

[Qais Yousef]

So that we can re-use them in other files.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
---
 include/sched_attr.h | 97 ++++++++++++++++++++++++++++++++++++++++++++
 schedutils/chrt.c    | 77 +----------------------------------
 2 files changed, 98 insertions(+), 76 deletions(-)
 create mode 100644 include/sched_attr.h

diff --git a/include/sched_attr.h b/include/sched_attr.h
new file mode 100644
index 000000000..b39d37b5d
--- /dev/null
+++ b/include/sched_attr.h
@@ -0,0 +1,97 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Copyright (C) 2004 Robert Love
+ */
+#ifndef UTIL_LINUX_SCHED_ATTR_H
+#define UTIL_LINUX_SCHED_ATTR_H
+
+/* the SCHED_BATCH is supported since Linux 2.6.16
+ *  -- temporary workaround for people with old glibc headers
+ */
+#if defined (__linux__) && !defined(SCHED_BATCH)
+# define SCHED_BATCH 3
+#endif
+
+/* the SCHED_IDLE is supported since Linux 2.6.23
+ * commit id 0e6aca43e08a62a48d6770e9a159dbec167bf4c6
+ * -- temporary workaround for people with old glibc headers
+ */
+#if defined (__linux__) && !defined(SCHED_IDLE)
+# define SCHED_IDLE 5
+#endif
+
+/* flag by sched_getscheduler() */
+#if defined(__linux__) && !defined(SCHED_RESET_ON_FORK)
+# define SCHED_RESET_ON_FORK 0x40000000
+#endif
+
+/* flag by sched_getattr() */
+#if defined(__linux__) && !defined(SCHED_FLAG_RESET_ON_FORK)
+# define SCHED_FLAG_RESET_ON_FORK 0x01
+#endif
+
+#if defined (__linux__)
+# include <sys/syscall.h>
+#endif
+
+/* usable kernel-headers, but old glibc-headers */
+#if defined (__linux__) && !defined(SYS_sched_setattr) && defined(__NR_sched_setattr)
+# define SYS_sched_setattr __NR_sched_setattr
+#endif
+
+#if defined (__linux__) && !defined(SYS_sched_getattr) && defined(__NR_sched_getattr)
+# define SYS_sched_getattr __NR_sched_getattr
+#endif
+
+#if defined (__linux__) && !defined(HAVE_SCHED_SETATTR) && defined(SYS_sched_setattr)
+# define HAVE_SCHED_SETATTR
+
+struct sched_attr {
+	uint32_t size;
+	uint32_t sched_policy;
+	uint64_t sched_flags;
+
+	/* SCHED_NORMAL, SCHED_BATCH */
+	int32_t sched_nice;
+
+	/* SCHED_FIFO, SCHED_RR */
+	uint32_t sched_priority;
+
+	/* SCHED_DEADLINE (nsec) */
+	uint64_t sched_runtime;
+	uint64_t sched_deadline;
+	uint64_t sched_period;
+};
+
+static int sched_setattr(pid_t pid, const struct sched_attr *attr, unsigned int flags)
+{
+	return syscall(SYS_sched_setattr, pid, attr, flags);
+}
+
+static int sched_getattr(pid_t pid, struct sched_attr *attr, unsigned int size, unsigned int flags)
+{
+	return syscall(SYS_sched_getattr, pid, attr, size, flags);
+}
+#endif
+
+/* the SCHED_DEADLINE is supported since Linux 3.14
+ * commit id aab03e05e8f7e26f51dee792beddcb5cca9215a5
+ * -- sched_setattr() is required for this policy!
+ */
+#if defined (__linux__) && !defined(SCHED_DEADLINE) && defined(HAVE_SCHED_SETATTR)
+# define SCHED_DEADLINE 6
+#endif
+
+#endif /* UTIL_LINUX_SCHED_ATTR_H */
diff --git a/schedutils/chrt.c b/schedutils/chrt.c
index 3a1608013..052ad7a1b 100644
--- a/schedutils/chrt.c
+++ b/schedutils/chrt.c
@@ -34,83 +34,8 @@
 #include "closestream.h"
 #include "strutils.h"
 #include "procutils.h"
+#include "sched_attr.h"
 
-/* the SCHED_BATCH is supported since Linux 2.6.16
- *  -- temporary workaround for people with old glibc headers
- */
-#if defined (__linux__) && !defined(SCHED_BATCH)
-# define SCHED_BATCH 3
-#endif
-
-/* the SCHED_IDLE is supported since Linux 2.6.23
- * commit id 0e6aca43e08a62a48d6770e9a159dbec167bf4c6
- * -- temporary workaround for people with old glibc headers
- */
-#if defined (__linux__) && !defined(SCHED_IDLE)
-# define SCHED_IDLE 5
-#endif
-
-/* flag by sched_getscheduler() */
-#if defined(__linux__) && !defined(SCHED_RESET_ON_FORK)
-# define SCHED_RESET_ON_FORK 0x40000000
-#endif
-
-/* flag by sched_getattr() */
-#if defined(__linux__) && !defined(SCHED_FLAG_RESET_ON_FORK)
-# define SCHED_FLAG_RESET_ON_FORK 0x01
-#endif
-
-#if defined (__linux__)
-# include <sys/syscall.h>
-#endif
-
-/* usable kernel-headers, but old glibc-headers */
-#if defined (__linux__) && !defined(SYS_sched_setattr) && defined(__NR_sched_setattr)
-# define SYS_sched_setattr __NR_sched_setattr
-#endif
-
-#if defined (__linux__) && !defined(SYS_sched_getattr) && defined(__NR_sched_getattr)
-# define SYS_sched_getattr __NR_sched_getattr
-#endif
-
-#if defined (__linux__) && !defined(HAVE_SCHED_SETATTR) && defined(SYS_sched_setattr)
-# define HAVE_SCHED_SETATTR
-
-struct sched_attr {
-	uint32_t size;
-	uint32_t sched_policy;
-	uint64_t sched_flags;
-
-	/* SCHED_NORMAL, SCHED_BATCH */
-	int32_t sched_nice;
-
-	/* SCHED_FIFO, SCHED_RR */
-	uint32_t sched_priority;
-
-	/* SCHED_DEADLINE (nsec) */
-	uint64_t sched_runtime;
-	uint64_t sched_deadline;
-	uint64_t sched_period;
-};
-
-static int sched_setattr(pid_t pid, const struct sched_attr *attr, unsigned int flags)
-{
-	return syscall(SYS_sched_setattr, pid, attr, flags);
-}
-
-static int sched_getattr(pid_t pid, struct sched_attr *attr, unsigned int size, unsigned int flags)
-{
-	return syscall(SYS_sched_getattr, pid, attr, size, flags);
-}
-#endif
-
-/* the SCHED_DEADLINE is supported since Linux 3.14
- * commit id aab03e05e8f7e26f51dee792beddcb5cca9215a5
- * -- sched_setattr() is required for this policy!
- */
-#if defined (__linux__) && !defined(SCHED_DEADLINE) && defined(HAVE_SCHED_SETATTR)
-# define SCHED_DEADLINE 6
-#endif
 
 /* control struct */
 struct chrt_ctl {
-- 
2.25.1

[PATCH 2/5] Add uclampset schedutil

[Qais Yousef]

Utilization clamping is a new kernel feature that got merged in 5.3. It
allows controlling the performance of a process by manipulating the
utilization such that the task appears bigger or smaller than what it
really is.

There's a system-wide control to to restrict what maximum values the
process are allowed to use.

Man page added in a later patch attempts to explain the usage in more
detail.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
---
 .gitignore             |   1 +
 include/sched_attr.h   |  30 ++++
 schedutils/uclampset.c | 378 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 409 insertions(+)
 create mode 100644 schedutils/uclampset.c

diff --git a/.gitignore b/.gitignore
index a6f379146..c01d2f644 100644
--- a/.gitignore
+++ b/.gitignore
@@ -180,3 +180,4 @@ ylwrap
 /wipefs
 /write
 /zramctl
+/uclampset
diff --git a/include/sched_attr.h b/include/sched_attr.h
index b39d37b5d..e8f8173ec 100644
--- a/include/sched_attr.h
+++ b/include/sched_attr.h
@@ -13,6 +13,8 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
  * Copyright (C) 2004 Robert Love
+ * Copyright (C) 2020 Qais Yousef
+ * Copyright (C) 2020 Arm Ltd
  */
 #ifndef UTIL_LINUX_SCHED_ATTR_H
 #define UTIL_LINUX_SCHED_ATTR_H
@@ -42,6 +44,30 @@
 # define SCHED_FLAG_RESET_ON_FORK 0x01
 #endif
 
+#if defined(__linux__) && !defined(SCHED_FLAG_RECLAIM)
+# define SCHED_FLAG_RECLAIM 0x02
+#endif
+
+#if defined(__linux__) && !defined(SCHED_FLAG_DL_OVERRUN)
+# define SCHED_FLAG_DL_OVERRUN 0x04
+#endif
+
+#if defined(__linux__) && !defined(SCHED_FLAG_KEEP_POLICY)
+# define SCHED_FLAG_KEEP_POLICY 0x08
+#endif
+
+#if defined(__linux__) && !defined(SCHED_FLAG_KEEP_PARAMS)
+# define SCHED_FLAG_KEEP_PARAMS 0x10
+#endif
+
+#if defined(__linux__) && !defined(SCHED_FLAG_UTIL_CLAMP_MIN)
+# define SCHED_FLAG_UTIL_CLAMP_MIN 0x20
+#endif
+
+#if defined(__linux__) && !defined(SCHED_FLAG_UTIL_CLAMP_MAX)
+# define SCHED_FLAG_UTIL_CLAMP_MAX 0x40
+#endif
+
 #if defined (__linux__)
 # include <sys/syscall.h>
 #endif
@@ -73,6 +99,10 @@ struct sched_attr {
 	uint64_t sched_runtime;
 	uint64_t sched_deadline;
 	uint64_t sched_period;
+
+	/* UTILIZATION CLAMPING */
+	uint32_t sched_util_min;
+	uint32_t sched_util_max;
 };
 
 static int sched_setattr(pid_t pid, const struct sched_attr *attr, unsigned int flags)
diff --git a/schedutils/uclampset.c b/schedutils/uclampset.c
new file mode 100644
index 000000000..d7d5619ad
--- /dev/null
+++ b/schedutils/uclampset.c
@@ -0,0 +1,378 @@
+/*
+ * uclampset.c - change utilization clamping attributes of a task or the system
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Copyright (C) 2020 Qais Yousef
+ * Copyright (C) 2020 Arm Ltd
+ */
+
+#include <errno.h>
+#include <getopt.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "closestream.h"
+#include "procutils.h"
+#include "sched_attr.h"
+#include "strutils.h"
+
+#define PROCFS(file)	"/proc/sys/kernel/" #file
+
+#define PROCFS_UCLAMP_MIN	PROCFS(sched_util_clamp_min)
+#define PROCFS_UCLAMP_MAX	PROCFS(sched_util_clamp_max)
+
+#define MAX_OPT		1000
+#define COMM_LEN	64
+#define NOT_SET		-1U
+
+struct uclampset {
+	unsigned int util_min;
+	unsigned int util_max;
+
+	pid_t pid;
+	unsigned int all_tasks;			/* all threads of the PID */
+
+	unsigned int system;
+
+	unsigned int verbose;
+};
+
+static void __attribute__((__noreturn__)) usage(void)
+{
+	FILE *out = stdout;
+
+	fputs(_("Show or change the utilization clamping attributes of a process or the system.\n"), out);
+	fputs(USAGE_SEPARATOR, out);
+	fputs(_("Set util clamp for a process:\n"
+	" uclampset [options] [-m <util_min>] [-M <util_max>] [cmd <arg>...]\n"
+	" uclampset [options] [-m <util_min>] [-M <util_max>] --pid <pid>\n"), out);
+	fputs(USAGE_SEPARATOR, out);
+	fputs(_("Get util clamp for a process:\n"
+	" uclampset [options] -p <pid>\n"), out);
+
+	fputs(USAGE_SEPARATOR, out);
+	fputs(_("Set util clamp for the sytem:\n"
+	" uclampset [options] --system [-m <util_min>] [-M <util_max>]\n"), out);
+	fputs(USAGE_SEPARATOR, out);
+	fputs(_("Get util clamp for the system:\n"
+	" uclampset [options] -s\n"), out);
+
+	fputs(USAGE_SEPARATOR, out);
+	fputs(_("Other options:\n"), out);
+	fputs(_(" -m                   util_min value to set\n"), out);
+	fputs(_(" -M                   util_max value to set\n"), out);
+	fputs(_(" -a, --all-tasks      operate on all the tasks (threads) for a given pid\n"), out);
+	fputs(_(" -p, --pid            operate on existing given pid\n"), out);
+	fputs(_(" -s, --system         operate on system\n"), out);
+	fputs(_(" --max                show min and max valid uclamp values\n"), out);
+	fputs(_(" -v, --verbose        display status information\n"), out);
+
+	fputs(USAGE_SEPARATOR, out);
+	printf(USAGE_HELP_OPTIONS(22));
+
+	printf(USAGE_MAN_TAIL("uclampset(1)"));
+	exit(EXIT_SUCCESS);
+}
+
+static void proc_pid_name(pid_t pid, char *name, int len)
+{
+	char *proc_comm_fmt = "/proc/%d/comm";
+	char proc_comm[COMM_LEN];
+	FILE *fp;
+	int size;
+
+	size = snprintf(proc_comm, COMM_LEN, proc_comm_fmt, pid);
+	if (size >= COMM_LEN || size < 0)
+		goto error;
+
+	fp = fopen(proc_comm, "r");
+	if (!fp)
+		goto error;
+
+	size = fread(name, 1, len, fp);
+	name[size-1] = '\0';
+
+	fclose(fp);
+
+	if (ferror(fp))
+		goto error;
+
+	return;
+error:
+	strncpy(name, "unknown", len);
+}
+
+static void show_uclamp_pid_info(pid_t pid)
+{
+#ifdef HAVE_SCHED_SETATTR
+	struct sched_attr sa;
+	char comm[COMM_LEN];
+
+	/* don't display "pid 0" as that is confusing */
+	if (!pid)
+		pid = getpid();
+
+
+	proc_pid_name(pid, comm, COMM_LEN);
+
+	if (sched_getattr(pid, &sa, sizeof(sa), 0) != 0)
+		err(EXIT_FAILURE, _("failed to get pid %d's uclamp values"), pid);
+
+	printf(_("%s-%d\n\tutil_min: %d\n\tutil_max: %d\n"),
+		  comm, pid, sa.sched_util_min, sa.sched_util_max);
+#else
+	err(EXIT_FAILURE, _("uclamp is not supported on this system"));
+#endif
+}
+
+static unsigned int read_uclamp_sysfs(char *dir)
+{
+	unsigned int size;
+	char buf[16];
+	FILE *fp;
+
+	fp = fopen(dir, "r");
+	if (!fp)
+		err(EXIT_FAILURE, _("cannot open %s"), dir);
+
+	size = fread(buf, 1, sizeof(buf), fp);
+	buf[size-1] = '\0';
+
+	if (ferror(fp)) {
+		fclose(fp);
+		err(EXIT_FAILURE, _("error writing %s"), dir);
+	}
+
+	fclose(fp);
+
+	return strtou32_or_err(buf, _("invalid util clamp value"));
+}
+
+static void write_uclamp_sysfs(char *dir, unsigned int val)
+{
+	unsigned int size;
+	char buf[16];
+	FILE *fp;
+
+	fp = fopen(dir, "w");
+	if (!fp)
+		err(EXIT_FAILURE, _("cannot open %s"), dir);
+
+	size = snprintf(buf, sizeof(buf), "%d", val);
+	buf[size] = '\n';
+	buf[size+1] = '\0';
+	fwrite(buf, 1, sizeof(buf), fp);
+
+	if (ferror(fp)) {
+		fclose(fp);
+		err(EXIT_FAILURE, _("error writing %s"), dir);
+	}
+
+	fclose(fp);
+}
+
+static void show_uclamp_system_info(void)
+{
+	unsigned int min, max;
+
+	min = read_uclamp_sysfs(PROCFS_UCLAMP_MIN);
+	max = read_uclamp_sysfs(PROCFS_UCLAMP_MAX);
+
+	printf(_("System\n\tutil_min: %u\n\tutil_max: %u\n"), min, max);
+}
+
+static void show_uclamp_info(struct uclampset *ctl)
+{
+	if (ctl->system) {
+		show_uclamp_system_info();
+	} else if (ctl->all_tasks) {
+		pid_t tid;
+		struct proc_tasks *ts = proc_open_tasks(ctl->pid);
+
+		if (!ts)
+			err(EXIT_FAILURE, _("cannot obtain the list of tasks"));
+
+		while (!proc_next_tid(ts, &tid))
+			show_uclamp_pid_info(tid);
+
+		proc_close_tasks(ts);
+	} else {
+		show_uclamp_pid_info(ctl->pid);
+	}
+}
+
+static void show_min_max(void)
+{
+	printf(_("util_min and util_max must be in the range of [0:1024] inclusive\n"));
+}
+
+#ifndef HAVE_SCHED_SETATTR
+static int set_uclamp_one(struct uclampset *ctl, pid_t pid)
+{
+	err(EXIT_FAILURE, _("uclamp is not supported on this system"));
+}
+
+#else /* !HAVE_SCHED_SETATTR */
+static int set_uclamp_one(struct uclampset *ctl, pid_t pid)
+{
+	struct sched_attr sa;
+
+	if (sched_getattr(pid, &sa, sizeof(sa), 0) != 0)
+		err(EXIT_FAILURE, _("failed to get pid %d's uclamp values"), pid);
+
+	if (ctl->util_min != NOT_SET)
+		sa.sched_util_min = ctl->util_min;
+	if (ctl->util_max != NOT_SET)
+		sa.sched_util_max = ctl->util_max;
+
+	sa.sched_flags = SCHED_FLAG_KEEP_POLICY |
+			 SCHED_FLAG_KEEP_PARAMS |
+			 SCHED_FLAG_UTIL_CLAMP_MIN |
+			 SCHED_FLAG_UTIL_CLAMP_MAX;
+
+	return sched_setattr(pid, &sa, 0);
+}
+#endif /* HAVE_SCHED_SETATTR */
+
+static void set_uclamp_pid(struct uclampset *ctl)
+{
+	if (ctl->all_tasks) {
+		pid_t tid;
+		struct proc_tasks *ts = proc_open_tasks(ctl->pid);
+
+		if (!ts)
+			err(EXIT_FAILURE, _("cannot obtain the list of tasks"));
+
+		while (!proc_next_tid(ts, &tid))
+			if (set_uclamp_one(ctl, tid) == -1)
+				err(EXIT_FAILURE, _("failed to set tid %d's uclamp values"), tid);
+
+		proc_close_tasks(ts);
+
+	} else if (set_uclamp_one(ctl, ctl->pid) == -1) {
+		err(EXIT_FAILURE, _("failed to set pid %d's uclamp values"), ctl->pid);
+	}
+}
+
+static void set_uclamp_system(struct uclampset *ctl)
+{
+	if (ctl->util_min == NOT_SET)
+		ctl->util_min = read_uclamp_sysfs(PROCFS_UCLAMP_MIN);
+
+	if (ctl->util_max == NOT_SET)
+		ctl->util_max = read_uclamp_sysfs(PROCFS_UCLAMP_MAX);
+
+	if (ctl->util_min > ctl->util_max) {
+		errno = EINVAL;
+		err(EXIT_FAILURE, _("util_min must be <= util_max"));
+	}
+
+	write_uclamp_sysfs(PROCFS_UCLAMP_MIN, ctl->util_min);
+	write_uclamp_sysfs(PROCFS_UCLAMP_MAX, ctl->util_max);
+}
+
+int main(int argc, char **argv)
+{
+	struct uclampset _ctl = {
+		.pid = -1,
+		.util_min = NOT_SET,
+		.util_max = NOT_SET
+	};
+	struct uclampset *ctl = &_ctl;
+	bool no_input;
+	int c;
+
+	static const struct option longopts[] = {
+		{ "all-tasks",  no_argument, NULL, 'a' },
+		{ "pid",	no_argument, NULL, 'p' },
+		{ "system",	no_argument, NULL, 's' },
+		{ "help",	no_argument, NULL, 'h' },
+		{ "max",        no_argument, NULL, MAX_OPT },
+		{ "verbose",	no_argument, NULL, 'v' },
+		{ "version",	no_argument, NULL, 'V' },
+		{ NULL,		no_argument, NULL, 0 }
+	};
+
+	setlocale(LC_ALL, "");
+	bindtextdomain(PACKAGE, LOCALEDIR);
+	textdomain(PACKAGE);
+	close_stdout_atexit();
+
+	while((c = getopt_long(argc, argv, "+asphmMvV", longopts, NULL)) != -1)
+	{
+		switch (c) {
+		case 'a':
+			ctl->all_tasks = 1;
+			break;
+		case MAX_OPT:
+			show_min_max();
+			return EXIT_SUCCESS;
+		case 'p':
+			errno = 0;
+			ctl->pid = strtos32_or_err(argv[optind], _("invalid PID argument"));
+			optind++;
+			break;
+		case 's':
+			ctl->system = 1;
+			break;
+		case 'v':
+			ctl->verbose = 1;
+			break;
+		case 'm':
+			ctl->util_min = strtos32_or_err(argv[optind], _("invalid util_min argument"));
+			optind++;
+			break;
+		case 'M':
+			ctl->util_max = strtos32_or_err(argv[optind], _("invalid util_max argument"));
+			optind++;
+			break;
+		case 'V':
+			print_version(EXIT_SUCCESS);
+			/* fallthrough */
+		case 'h':
+			usage();
+		default:
+			errtryhelp(EXIT_FAILURE);
+		}
+	}
+
+	no_input = ctl->util_min == NOT_SET && ctl->util_max == NOT_SET;
+
+	if (no_input) {
+		show_uclamp_info(ctl);
+		return EXIT_SUCCESS;
+	}
+
+	if (ctl->pid == -1)
+		ctl->pid = 0;
+
+	if (ctl->system)
+		set_uclamp_system(ctl);
+	else
+		set_uclamp_pid(ctl);
+
+	if (ctl->verbose)
+		show_uclamp_info(ctl);
+
+	if (!ctl->pid && !ctl->system) {
+		argv += optind;
+		execvp(argv[0], argv);
+		errexec(argv[0]);
+	}
+
+	return EXIT_SUCCESS;
+}
-- 
2.25.1

[PATCH 3/5] uclampset: Add man page

[Qais Yousef]

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
---
 schedutils/uclampset.1 | 169 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)
 create mode 100644 schedutils/uclampset.1

diff --git a/schedutils/uclampset.1 b/schedutils/uclampset.1
new file mode 100644
index 000000000..1a525458a
--- /dev/null
+++ b/schedutils/uclampset.1
@@ -0,0 +1,169 @@
+.\" uclampset(1) manpage
+.\"
+.\" Copyright (C) 2020 Qais Yousef <qais.yousef@arm.com>
+.\" Copyright (C) 2020 Arm Ltd
+.\"
+.\" This is free documentation; you can redistribute it and/or
+.\" modify it under the terms of the GNU General Public License,
+.\" version 2, as published by the Free Software Foundation.
+.\"
+.\" The GNU General Public License's references to "object code"
+.\" and "executables" are to be interpreted as the output of any
+.\" document formatting or typesetting system, including
+.\" intermediate and printed output.
+.\"
+.\" This manual is distributed in the hope that it will be useful,
+.\" but WITHOUT ANY WARRANTY; without even the implied warranty of
+.\" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+.\" GNU General Public License for more details.
+.\"
+.\" You should have received a copy of the GNU General Public License along
+.\" with this program; if not, write to the Free Software Foundation, Inc.,
+.\" 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+.\"
+.TH UCLAMPSET 1 "August 2020" "util-linux" "User Commands"
+.SH NAME
+uclampset \- manipulate the utilization clamping attributes of the system or
+a process.
+.SH SYNOPSIS
+.B uclampset
+[options]
+.RI [ -m\ uclamp_min ]\ [ -M\ uclamp_max ]\ command\  [ argument ...]
+.br
+.B uclampset
+[options]
+.RI [ -m\ uclamp_min ]\ [ -M\ uclamp_max ]
+.B \-p
+.RI pid
+.SH DESCRIPTION
+.B uclampset
+sets or retrieves the utilization clamping attributes of an existing \fIpid\fR,
+or runs \fIcommand\fR with the given attributes.
+
+Utilization clamping is a new feature added in v5.3. It gives a hint to the
+scheduler about the allowed range of utilization the task should be operating
+at.
+
+The utilization of the task affects frequency selection and task placement.
+Only schedutil cpufreq governor understands handling util clamp hints at the
+time of writing. Consult your kernel docs for further info about other
+cpufreq governors support.
+
+If you're running on asymmetric heterogeneous system like Arm's big.LITTLE.
+Utilization clamping can help bias task placement. If the task is boosted such
+that
+.BR util_min
+value is higher than the little cores' capacity, then the scheduler will do its
+best to place it on a big core.
+
+Similarly, if
+.BR util_max
+is smaller than or equal the capacity of the little cores, then the scheduler
+can still choose to place it there even if the actual utilization of the task
+is at max.
+
+Setting a task's
+.B uclamp_min
+to a none zero value  will effectively boost the task as when it runs it'll
+always start from this utilization value.
+
+By setting a task's
+.B uclamp_max
+below 1024, this will effectively cap the task as when it runs it'll never be
+able to go above this utilization value.
+
+The full utilization range is: [0:1024].
+
+.SH OPTIONS
+.TP
+.BR \-m
+Set util_min value.
+.TP
+.BR \-M
+Set util_max value.
+.TP
+.BR \-a ,\  \-\-all-tasks
+Set or retrieve the utilization clamping attributes of all the tasks (threads)
+for a given PID.
+.TP
+.BR \-s ,\  \-\-system
+Set or retrieve the system-wide utilization clamping attributes.
+.TP
+.BR \-\-max
+Show minimum and maximum valid utilization values, then exit.
+.TP
+.BR \-p ,\  \-\-pid
+Operate on an existing PID and do not launch a new task.
+.TP
+.BR \-v ,\  \-\-verbose
+Show status information.
+.TP
+.BR \-V ,\  \-\-version
+Display version information and exit.
+.TP
+.BR \-h ,\  \-\-help
+Display help text and exit.
+.SH USAGE
+.TP
+The default behavior is to run a new command:
+.B uclampset
+.I [-m\ uclamp_min]
+.I [-M\ uclamp_max]
+.IR command\  [ arguments ]
+.TP
+You can also retrieve the utilization clamping attributes of an existing task:
+.B uclampset \-p
+.I pid
+.TP
+Or set them:
+.B uclampset \-p
+.I pid
+.I [-m\ uclamp_min]
+.I [-M\ uclamp_max]
+.TP
+Or control the system-wide attributes:
+.B uclampset \-s
+.I [-m\ uclamp_min]
+.I [-M\ uclamp_max]
+.SH PERMISSIONS
+A user must possess
+.B CAP_SYS_NICE
+to change the scheduling attributes of a process.  Any user can retrieve the
+scheduling information.
+
+.SH NOTES
+The system wide utilization clamp attributes are there to control the _allowed_
+range the tasks can use. By default both
+.BR uclamp_min
+and
+.BR uclamp_max
+are set to 1024. This means users can set the utilization clamp values for
+their task across the full range [0:1024].
+
+.TP
+For example:
+.B uclampset \-s
+.I -m\ 512
+.I -M\ 700
+.PP
+will prevent any task from being boosted higher than 512. And all tasks in the
+systems are capped to a utilization of 700. Effectively rendering the maximum
+performance of the system to 700.
+
+Consult your kernel docs for the exact expected behavior on that kernel.
+.SH AUTHORS
+.UR qais.yousef@arm.com
+Qais Yousef
+.UE
+.SH SEE ALSO
+.BR nice (1),
+.BR renice (1),
+.BR taskset (1),
+.BR sched (7)
+.sp
+See
+.BR sched_setscheduler (2)
+for a description of the Linux scheduling scheme.
+.SH AVAILABILITY
+The uclampset command is part of the util-linux package and is available from
+https://www.kernel.org/pub/linux/utils/util-linux/.
-- 
2.25.1

[PATCH 4/5] uclampset: Plump into the build system

[Qais Yousef]

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
---
 configure.ac             | 9 +++++++++
 schedutils/Makemodule.am | 7 +++++++
 2 files changed, 16 insertions(+)

diff --git a/configure.ac b/configure.ac
index 20b6c3178..8f66d5ec4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2194,6 +2194,15 @@ AS_IF([test "x$build_chrt" = xyes], [
 	UL_CHECK_SYSCALL([sched_setattr])
 ])
 
+UL_ENABLE_ALIAS([uclampset], [schedutils])
+UL_BUILD_INIT([uclampset])
+UL_REQUIRES_HAVE([uclampset], [schedsetter], [sched_set functions])
+AM_CONDITIONAL([BUILD_UCLAMPSET], [test "x$build_uclampset" = xyes])
+
+AS_IF([test "x$build_uclampset" = xyes], [
+	UL_CHECK_SYSCALL([sched_setattr])
+])
+
 
 AC_ARG_ENABLE([wall],
   AS_HELP_STRING([--disable-wall], [do not build wall]),
diff --git a/schedutils/Makemodule.am b/schedutils/Makemodule.am
index f32d2b307..c781ede63 100644
--- a/schedutils/Makemodule.am
+++ b/schedutils/Makemodule.am
@@ -18,3 +18,10 @@ dist_man_MANS += schedutils/taskset.1
 taskset_SOURCES = schedutils/taskset.c
 taskset_LDADD = $(LDADD) libcommon.la
 endif
+
+if BUILD_UCLAMPSET
+usrbin_exec_PROGRAMS += uclampset
+dist_man_MANS += schedutils/uclampset.1
+uclampset_SOURCES = schedutils/uclampset.c
+uclampset_LDADD = $(LDADD) libcommon.la
+endif
-- 
2.25.1

[PATCH 5/5] uclampset: Plumb in bash-completion

[Qais Yousef]

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
---
 bash-completion/Makemodule.am |  3 +++
 bash-completion/uclampset     | 39 +++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 bash-completion/uclampset

diff --git a/bash-completion/Makemodule.am b/bash-completion/Makemodule.am
index b80c23f7b..3384ba4e2 100644
--- a/bash-completion/Makemodule.am
+++ b/bash-completion/Makemodule.am
@@ -210,6 +210,9 @@ endif
 if BUILD_CHRT
 dist_bashcompletion_DATA += bash-completion/chrt
 endif
+if BUILD_UCLAMPSET
+dist_bashcompletion_DATA += bash-completion/uclampset
+endif
 if BUILD_IONICE
 dist_bashcompletion_DATA += bash-completion/ionice
 endif
diff --git a/bash-completion/uclampset b/bash-completion/uclampset
new file mode 100644
index 000000000..44ae80365
--- /dev/null
+++ b/bash-completion/uclampset
@@ -0,0 +1,39 @@
+_uclampset_module()
+{
+	local cur prev OPTS
+	COMPREPLY=()
+	cur="${COMP_WORDS[COMP_CWORD]}"
+	prev="${COMP_WORDS[COMP_CWORD-1]}"
+	case $prev in
+		'-h'|'--help'|'-V'|'--version')
+			return 0
+			;;
+	esac
+	case $cur in
+		-*)
+			OPTS="
+				--all-tasks
+				--help
+				--max
+				--pid
+				--system
+				--verbose
+				--version
+			"
+			COMPREPLY=( $(compgen -W "${OPTS[*]}" -- $cur) )
+			return 0
+			;;
+	esac
+	local i
+	for i in ${COMP_WORDS[*]}; do
+		case $i in
+		'-p'|'--pid')
+			COMPREPLY=( $(compgen -W "$(cd /proc && echo [0-9]*)" -- $cur) )
+			return 0
+			;;
+		esac
+	done
+	COMPREPLY=( $(compgen -c -- $cur) )
+	return 0
+}
+complete -F _uclampset_module uclampset
-- 
2.25.1

Re: [PATCH 0/5] Add a new uclampset utility

[Qais Yousef]

On 01/16/21 19:09, Qais Yousef wrote:
> Since kernel v5.3 we have a new feature called utilization clamping that allows
> influencing the utilization signals of a task, ultimately influencing the
> performance of these tasks.
> 
> The series adds a new utility called uclampset that allows the user to
> manipulate util clamp (or uclamp for short) for existing running processes or
> when running a new command; in a similar spirit to how taskset and chrt
> currently work.
> 
> Peter/Dietmar/Vincent/Patrick; reviewing the manpage (patch 3) to make sure it
> explains this feature right would be much appreciated.

You can find a branch of the changes in my github too if it helps:

	git clone https://github.com/qais-yousef/util-linux -b uclampset

Thanks

--
Qais Yousef

Re: [PATCH 1/5] Move sched_attr struct and syscall definitions into header file

[Karel Zak]

On Sat, Jan 16, 2021 at 07:09:36PM +0000, Qais Yousef wrote:
> So that we can re-use them in other files.

Good idea.

> +++ b/include/sched_attr.h
> @@ -0,0 +1,97 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation

My goal is to keep include/ and lib/ usable for LGPL and BRD code,
because util-linux is not only GPL code. The ideal solution is to 
use public domain or LGPL for this shared directories. 

Maybe we can use Public Domain also for this new header file as it
contains only ifdef fallbacks. 

The other solution is to keep GPL sched_attr.h in schedutils/ directory.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 2/5] Add uclampset schedutil

[Karel Zak]

 Hi,

pretty readable code (thanks), I have some notes:

On Sat, Jan 16, 2021 at 07:09:37PM +0000, Qais Yousef wrote:
>  #if defined (__linux__)
>  # include <sys/syscall.h>
>  #endif
> @@ -73,6 +99,10 @@ struct sched_attr {
>  	uint64_t sched_runtime;
>  	uint64_t sched_deadline;
>  	uint64_t sched_period;
> +
> +	/* UTILIZATION CLAMPING */
> +	uint32_t sched_util_min;
> +	uint32_t sched_util_max;
>  };

This sched_attr definition is used only if regular system include files
do not contain struct sched_attr (aka ifndef HAVE_SCHED_SETATTR).

What is expected on systems with header files from Linux < 5.3 where
is sched_attr, but without sched_util_{min,max}?

I guess you need to check in ./configure.ac that struct sched_attr is
usable for your uclampset otherwise you will see compilation error.
You can use for example AC_LINK_IFELSE([AC_LANG_PROGRAM([[ ... and
try to compile code with sched_attr.sched_util_min = 0.

I see two possible ways:

- do not compile uclampset at all if there is no sched_attr.sched_util_*
  in system header files

- use you local definition from sched_attr.h, but it will probably
  require to define own struct to avoid names collision

IMHO the first solution is more simple and it will keep your code readable.

If you not sure with the build-system stuff then ignore it and don't
waste time with it. I can fix it before merge into master branch.

> +#define PROCFS(file)	"/proc/sys/kernel/" #file
> +
> +#define PROCFS_UCLAMP_MIN	PROCFS(sched_util_clamp_min)
> +#define PROCFS_UCLAMP_MAX	PROCFS(sched_util_clamp_max)

Please, add it to include/pathnames.h as
 
 #define _PATH_PROC_KERNEL      "/proc/sys/kernel"
 #define _PATH_PROC_UCLAMP_MIN  PATH_PROC_KERNEL "/sched_util_clamp_min"
 #defene _PATH_PROC_UCLAMP_MAN  PATH_PROC_KERNEL "/sched_util_clamp_max"


> +#define MAX_OPT		1000
> +#define COMM_LEN	64
> +#define NOT_SET		-1U
> +
> +struct uclampset {
> +	unsigned int util_min;
> +	unsigned int util_max;
> +
> +	pid_t pid;
> +	unsigned int all_tasks;			/* all threads of the PID */
> +
> +	unsigned int system;
> +
> +	unsigned int verbose;

Seems like we can use bits only:

    unsigned int all_tasks:1
                 system:1,
                 verbose:1;

> +static void __attribute__((__noreturn__)) usage(void)
> +{
> +	FILE *out = stdout;
> +
> +	fputs(_("Show or change the utilization clamping attributes of a process or the system.\n"), out);

We usually have this description after synopsis and before "Options".

> +	fputs(USAGE_SEPARATOR, out);
> +	fputs(_("Set util clamp for a process:\n"
> +	" uclampset [options] [-m <util_min>] [-M <util_max>] [cmd <arg>...]\n"
> +	" uclampset [options] [-m <util_min>] [-M <util_max>] --pid <pid>\n"), out);
> +	fputs(USAGE_SEPARATOR, out);
> +	fputs(_("Get util clamp for a process:\n"
> +	" uclampset [options] -p <pid>\n"), out);
> +
> +	fputs(USAGE_SEPARATOR, out);
> +	fputs(_("Set util clamp for the sytem:\n"
> +	" uclampset [options] --system [-m <util_min>] [-M <util_max>]\n"), out);
> +	fputs(USAGE_SEPARATOR, out);
> +	fputs(_("Get util clamp for the system:\n"
> +	" uclampset [options] -s\n"), out);

You do not have to be so verbose :-)

        fprintf(out,
              _(" %1$s [options]\n"
                " %1$s [options] --pid <pid> | --system | <command> <arg>...\n",
                program_invocation_short_name);
seems enough.

> +	fputs(USAGE_SEPARATOR, out);
> +	fputs(_("Other options:\n"), out);

fputs(USAGE_OPTIONS, out);

> +	fputs(_(" -m                   util_min value to set\n"), out);
> +	fputs(_(" -M                   util_max value to set\n"), out);
> +	fputs(_(" -a, --all-tasks      operate on all the tasks (threads) for a given pid\n"), out);
> +	fputs(_(" -p, --pid            operate on existing given pid\n"), out);
> +	fputs(_(" -s, --system         operate on system\n"), out);
> +	fputs(_(" --max                show min and max valid uclamp values\n"), out);
> +	fputs(_(" -v, --verbose        display status information\n"), out);
> +
> +	fputs(USAGE_SEPARATOR, out);
> +	printf(USAGE_HELP_OPTIONS(22));
> +
> +	printf(USAGE_MAN_TAIL("uclampset(1)"));
> +	exit(EXIT_SUCCESS);
> +}
> +
> +static void proc_pid_name(pid_t pid, char *name, int len)
> +{
> +	char *proc_comm_fmt = "/proc/%d/comm";
> +	char proc_comm[COMM_LEN];
> +	FILE *fp;
> +	int size;
> +
> +	size = snprintf(proc_comm, COMM_LEN, proc_comm_fmt, pid);
> +	if (size >= COMM_LEN || size < 0)
> +		goto error;
> +
> +	fp = fopen(proc_comm, "r");
> +	if (!fp)
> +		goto error;
> +
> +	size = fread(name, 1, len, fp);
> +	name[size-1] = '\0';
> +
> +	fclose(fp);
> +
> +	if (ferror(fp))
> +		goto error;
> +
> +	return;
> +error:
> +	strncpy(name, "unknown", len);
> +}

Please, use proc_get_command_name() from lib/procutils.c 

> +static void show_uclamp_pid_info(pid_t pid)
> +{
> +#ifdef HAVE_SCHED_SETATTR

This is unnecessary #ifdef, we will not compile uclampset at all if
sched_getattr() and sched_setattr() are not available.

> +	struct sched_attr sa;
> +	char comm[COMM_LEN];
> +
> +	/* don't display "pid 0" as that is confusing */
> +	if (!pid)
> +		pid = getpid();
> +
> +
> +	proc_pid_name(pid, comm, COMM_LEN);
> +
> +	if (sched_getattr(pid, &sa, sizeof(sa), 0) != 0)
> +		err(EXIT_FAILURE, _("failed to get pid %d's uclamp values"), pid);
> +
> +	printf(_("%s-%d\n\tutil_min: %d\n\tutil_max: %d\n"),
> +		  comm, pid, sa.sched_util_min, sa.sched_util_max);

  comm =  proc_get_command_name(pid);

  printf(_("%s-%d\n\tutil_min: %d\n\tutil_max: %d\n"),
          comm ? : "uknown", pid, sa.sched_util_min, sa.sched_util_max);
  
  free(comm);

I like the idea with command name, it seems more user-friendly
than chrt-way:  "pid 876909's current  ...", but I'm not sure with
multiple lines for each PID (%s-%d\n\t)

Let's imagine you will use

  uclamp --all-tasks --pid 123

in your script. It would be probably better to use

   "%s (%d) utilization: min: %d, max: %d\n"

to keep it easy to parse.

> +#else
> +	err(EXIT_FAILURE, _("uclamp is not supported on this system"));
> +#endif
> +}
> +
> +static unsigned int read_uclamp_sysfs(char *dir)

Please, s/dir/filename/

> +{
> +	unsigned int size;
> +	char buf[16];
> +	FILE *fp;
> +
> +	fp = fopen(dir, "r");
> +	if (!fp)
> +		err(EXIT_FAILURE, _("cannot open %s"), dir);
> +
> +	size = fread(buf, 1, sizeof(buf), fp);
> +	buf[size-1] = '\0';
> +
> +	if (ferror(fp)) {
> +		fclose(fp);
> +		err(EXIT_FAILURE, _("error writing %s"), dir);

 s/error writing %s/cannot read %s/

> +	}
> +
> +	fclose(fp);
> +
> +	return strtou32_or_err(buf, _("invalid util clamp value"));

you can replace all this code with stuff from lib/path.c

  unsigned int res;

  if (ul_path_read_u32(NULL, &res, filename) != 0)
    err(EXIT_FAILURE, _("cannot read %s), filename);

> +}
> +
> +static void write_uclamp_sysfs(char *dir, unsigned int val)
> +{
> +	unsigned int size;
> +	char buf[16];
> +	FILE *fp;
> +
> +	fp = fopen(dir, "w");
> +	if (!fp)
> +		err(EXIT_FAILURE, _("cannot open %s"), dir);
> +
> +	size = snprintf(buf, sizeof(buf), "%d", val);
> +	buf[size] = '\n';
> +	buf[size+1] = '\0';
> +	fwrite(buf, 1, sizeof(buf), fp);
> +
> +	if (ferror(fp)) {
> +		fclose(fp);
> +		err(EXIT_FAILURE, _("error writing %s"), dir);

"cannot write %s" 

If not sure see po/util-linux.pot and use the most common string. It
will save translators' time ;-)

> +	}
> +
> +	fclose(fp);
> +}

but, you want to use:  ul_path_write_u64(NULL, val, filename)

> +static void show_uclamp_system_info(void)
> +{
> +	unsigned int min, max;
> +
> +	min = read_uclamp_sysfs(PROCFS_UCLAMP_MIN);
> +	max = read_uclamp_sysfs(PROCFS_UCLAMP_MAX);
> +
> +	printf(_("System\n\tutil_min: %u\n\tutil_max: %u\n"), min, max);
> +}
> +
> +static void show_uclamp_info(struct uclampset *ctl)
> +{
> +	if (ctl->system) {
> +		show_uclamp_system_info();
> +	} else if (ctl->all_tasks) {
> +		pid_t tid;
> +		struct proc_tasks *ts = proc_open_tasks(ctl->pid);
> +
> +		if (!ts)
> +			err(EXIT_FAILURE, _("cannot obtain the list of tasks"));
> +
> +		while (!proc_next_tid(ts, &tid))
> +			show_uclamp_pid_info(tid);
> +
> +		proc_close_tasks(ts);
> +	} else {
> +		show_uclamp_pid_info(ctl->pid);
> +	}
> +}
> +
> +static void show_min_max(void)
> +{
> +	printf(_("util_min and util_max must be in the range of [0:1024] inclusive\n"));
> +}

Why we need --max to get hardcode range? Would be enough to add this
note to --usage output?

> +#ifndef HAVE_SCHED_SETATTR
> +static int set_uclamp_one(struct uclampset *ctl, pid_t pid)
> +{
> +	err(EXIT_FAILURE, _("uclamp is not supported on this system"));
> +}

Again, unnecessary #ifndef.

> +#else /* !HAVE_SCHED_SETATTR */
> +static int set_uclamp_one(struct uclampset *ctl, pid_t pid)
> +{
> +	struct sched_attr sa;
> +
> +	if (sched_getattr(pid, &sa, sizeof(sa), 0) != 0)
> +		err(EXIT_FAILURE, _("failed to get pid %d's uclamp values"), pid);
> +
> +	if (ctl->util_min != NOT_SET)
> +		sa.sched_util_min = ctl->util_min;
> +	if (ctl->util_max != NOT_SET)
> +		sa.sched_util_max = ctl->util_max;
> +
> +	sa.sched_flags = SCHED_FLAG_KEEP_POLICY |
> +			 SCHED_FLAG_KEEP_PARAMS |
> +			 SCHED_FLAG_UTIL_CLAMP_MIN |
> +			 SCHED_FLAG_UTIL_CLAMP_MAX;
> +
> +	return sched_setattr(pid, &sa, 0);
> +}

What about SCHED_FLAG_RESET_ON_FORK flag (aka -R for chrt(8))? Does it
matter for the task utilization attributes?

> +#endif /* HAVE_SCHED_SETATTR */
> +
> +static void set_uclamp_pid(struct uclampset *ctl)
> +{
> +	if (ctl->all_tasks) {
> +		pid_t tid;
> +		struct proc_tasks *ts = proc_open_tasks(ctl->pid);
> +
> +		if (!ts)
> +			err(EXIT_FAILURE, _("cannot obtain the list of tasks"));
> +
> +		while (!proc_next_tid(ts, &tid))
> +			if (set_uclamp_one(ctl, tid) == -1)
> +				err(EXIT_FAILURE, _("failed to set tid %d's uclamp values"), tid);
> +
> +		proc_close_tasks(ts);
> +
> +	} else if (set_uclamp_one(ctl, ctl->pid) == -1) {
> +		err(EXIT_FAILURE, _("failed to set pid %d's uclamp values"), ctl->pid);
> +	}
> +}
> +
> +static void set_uclamp_system(struct uclampset *ctl)
> +{
> +	if (ctl->util_min == NOT_SET)
> +		ctl->util_min = read_uclamp_sysfs(PROCFS_UCLAMP_MIN);
> +
> +	if (ctl->util_max == NOT_SET)
> +		ctl->util_max = read_uclamp_sysfs(PROCFS_UCLAMP_MAX);
> +
> +	if (ctl->util_min > ctl->util_max) {
> +		errno = EINVAL;
> +		err(EXIT_FAILURE, _("util_min must be <= util_max"));
> +	}
> +
> +	write_uclamp_sysfs(PROCFS_UCLAMP_MIN, ctl->util_min);
> +	write_uclamp_sysfs(PROCFS_UCLAMP_MAX, ctl->util_max);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	struct uclampset _ctl = {
> +		.pid = -1,
> +		.util_min = NOT_SET,
> +		.util_max = NOT_SET
> +	};
> +	struct uclampset *ctl = &_ctl;
> +	bool no_input;
> +	int c;
> +
> +	static const struct option longopts[] = {
> +		{ "all-tasks",  no_argument, NULL, 'a' },
> +		{ "pid",	no_argument, NULL, 'p' },
> +		{ "system",	no_argument, NULL, 's' },
> +		{ "help",	no_argument, NULL, 'h' },
> +		{ "max",        no_argument, NULL, MAX_OPT },
> +		{ "verbose",	no_argument, NULL, 'v' },
> +		{ "version",	no_argument, NULL, 'V' },
> +		{ NULL,		no_argument, NULL, 0 }
> +	};
> +
> +	setlocale(LC_ALL, "");
> +	bindtextdomain(PACKAGE, LOCALEDIR);
> +	textdomain(PACKAGE);
> +	close_stdout_atexit();
> +
> +	while((c = getopt_long(argc, argv, "+asphmMvV", longopts, NULL)) != -1)
> +	{
> +		switch (c) {
> +		case 'a':
> +			ctl->all_tasks = 1;
> +			break;
> +		case MAX_OPT:
> +			show_min_max();
> +			return EXIT_SUCCESS;
> +		case 'p':
> +			errno = 0;
> +			ctl->pid = strtos32_or_err(argv[optind], _("invalid PID argument"));
> +			optind++;
> +			break;
> +		case 's':
> +			ctl->system = 1;
> +			break;
> +		case 'v':
> +			ctl->verbose = 1;
> +			break;
> +		case 'm':
> +			ctl->util_min = strtos32_or_err(argv[optind], _("invalid util_min argument"));
> +			optind++;
> +			break;
> +		case 'M':
> +			ctl->util_max = strtos32_or_err(argv[optind], _("invalid util_max argument"));
> +			optind++;
> +			break;
> +		case 'V':
> +			print_version(EXIT_SUCCESS);
> +			/* fallthrough */
> +		case 'h':
> +			usage();
> +		default:
> +			errtryhelp(EXIT_FAILURE);
> +		}
> +	}
> +
> +	no_input = ctl->util_min == NOT_SET && ctl->util_max == NOT_SET;
> +
> +	if (no_input) {
> +		show_uclamp_info(ctl);
> +		return EXIT_SUCCESS;
> +	}
> +
> +	if (ctl->pid == -1)
> +		ctl->pid = 0;
> +
> +	if (ctl->system)
> +		set_uclamp_system(ctl);
> +	else
> +		set_uclamp_pid(ctl);
> +
> +	if (ctl->verbose)
> +		show_uclamp_info(ctl);
> +
> +	if (!ctl->pid && !ctl->system) {
> +		argv += optind;
> +		execvp(argv[0], argv);
> +		errexec(argv[0]);
> +	}
> +
> +	return EXIT_SUCCESS;
> +}

Thanks!

 Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 3/5] uclampset: Add man page

[Karel Zak]

On Sat, Jan 16, 2021 at 07:09:38PM +0000, Qais Yousef wrote:
> +See
> +.BR sched_setscheduler (2)
> +for a description of the Linux scheduling scheme.

and maybe sched_setattr(2) too.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 3/5] uclampset: Add man page

[Vincent Guittot]

On Sat, 16 Jan 2021 at 20:11, Qais Yousef <qais.yousef@arm.com> wrote:
>
> Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> ---
>  schedutils/uclampset.1 | 169 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 169 insertions(+)
>  create mode 100644 schedutils/uclampset.1
>
> diff --git a/schedutils/uclampset.1 b/schedutils/uclampset.1
> new file mode 100644
> index 000000000..1a525458a
> --- /dev/null
> +++ b/schedutils/uclampset.1
> @@ -0,0 +1,169 @@
> +.\" uclampset(1) manpage
> +.\"
> +.\" Copyright (C) 2020 Qais Yousef <qais.yousef@arm.com>
> +.\" Copyright (C) 2020 Arm Ltd
> +.\"
> +.\" This is free documentation; you can redistribute it and/or
> +.\" modify it under the terms of the GNU General Public License,
> +.\" version 2, as published by the Free Software Foundation.
> +.\"
> +.\" The GNU General Public License's references to "object code"
> +.\" and "executables" are to be interpreted as the output of any
> +.\" document formatting or typesetting system, including
> +.\" intermediate and printed output.
> +.\"
> +.\" This manual is distributed in the hope that it will be useful,
> +.\" but WITHOUT ANY WARRANTY; without even the implied warranty of
> +.\" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +.\" GNU General Public License for more details.
> +.\"
> +.\" You should have received a copy of the GNU General Public License along
> +.\" with this program; if not, write to the Free Software Foundation, Inc.,
> +.\" 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> +.\"
> +.TH UCLAMPSET 1 "August 2020" "util-linux" "User Commands"
> +.SH NAME
> +uclampset \- manipulate the utilization clamping attributes of the system or
> +a process.
> +.SH SYNOPSIS
> +.B uclampset
> +[options]
> +.RI [ -m\ uclamp_min ]\ [ -M\ uclamp_max ]\ command\  [ argument ...]
> +.br
> +.B uclampset
> +[options]
> +.RI [ -m\ uclamp_min ]\ [ -M\ uclamp_max ]
> +.B \-p
> +.RI pid
> +.SH DESCRIPTION
> +.B uclampset
> +sets or retrieves the utilization clamping attributes of an existing \fIpid\fR,
> +or runs \fIcommand\fR with the given attributes.
> +
> +Utilization clamping is a new feature added in v5.3. It gives a hint to the
> +scheduler about the allowed range of utilization the task should be operating
> +at.
> +
> +The utilization of the task affects frequency selection and task placement.
> +Only schedutil cpufreq governor understands handling util clamp hints at the
> +time of writing. Consult your kernel docs for further info about other
> +cpufreq governors support.
> +
> +If you're running on asymmetric heterogeneous system like Arm's big.LITTLE.
> +Utilization clamping can help bias task placement. If the task is boosted such
> +that
> +.BR util_min
> +value is higher than the little cores' capacity, then the scheduler will do its
> +best to place it on a big core.
> +
> +Similarly, if
> +.BR util_max
> +is smaller than or equal the capacity of the little cores, then the scheduler
> +can still choose to place it there even if the actual utilization of the task
> +is at max.
> +
> +Setting a task's
> +.B uclamp_min
> +to a none zero value  will effectively boost the task as when it runs it'll
> +always start from this utilization value.
> +
> +By setting a task's
> +.B uclamp_max
> +below 1024, this will effectively cap the task as when it runs it'll never be
> +able to go above this utilization value.
> +
> +The full utilization range is: [0:1024].
> +
> +.SH OPTIONS
> +.TP
> +.BR \-m
> +Set util_min value.
> +.TP
> +.BR \-M
> +Set util_max value.
> +.TP
> +.BR \-a ,\  \-\-all-tasks
> +Set or retrieve the utilization clamping attributes of all the tasks (threads)
> +for a given PID.
> +.TP
> +.BR \-s ,\  \-\-system
> +Set or retrieve the system-wide utilization clamping attributes.
> +.TP
> +.BR \-\-max
> +Show minimum and maximum valid utilization values, then exit.
> +.TP
> +.BR \-p ,\  \-\-pid
> +Operate on an existing PID and do not launch a new task.
> +.TP
> +.BR \-v ,\  \-\-verbose
> +Show status information.
> +.TP
> +.BR \-V ,\  \-\-version
> +Display version information and exit.
> +.TP
> +.BR \-h ,\  \-\-help
> +Display help text and exit.
> +.SH USAGE
> +.TP
> +The default behavior is to run a new command:
> +.B uclampset
> +.I [-m\ uclamp_min]
> +.I [-M\ uclamp_max]
> +.IR command\  [ arguments ]
> +.TP
> +You can also retrieve the utilization clamping attributes of an existing task:
> +.B uclampset \-p
> +.I pid
> +.TP
> +Or set them:
> +.B uclampset \-p
> +.I pid
> +.I [-m\ uclamp_min]
> +.I [-M\ uclamp_max]
> +.TP
> +Or control the system-wide attributes:
> +.B uclampset \-s
> +.I [-m\ uclamp_min]
> +.I [-M\ uclamp_max]
> +.SH PERMISSIONS
> +A user must possess
> +.B CAP_SYS_NICE
> +to change the scheduling attributes of a process.  Any user can retrieve the
> +scheduling information.
> +
> +.SH NOTES
> +The system wide utilization clamp attributes are there to control the _allowed_
> +range the tasks can use. By default both
> +.BR uclamp_min
> +and
> +.BR uclamp_max
> +are set to 1024. This means users can set the utilization clamp values for
> +their task across the full range [0:1024].
> +
> +.TP
> +For example:
> +.B uclampset \-s
> +.I -m\ 512
> +.I -M\ 700
> +.PP
> +will prevent any task from being boosted higher than 512. And all tasks in the

will prevent any task from being boosted higher than 700
or
will ensure  any task to be boosted higher than 512


> +systems are capped to a utilization of 700. Effectively rendering the maximum
> +performance of the system to 700.
> +
> +Consult your kernel docs for the exact expected behavior on that kernel.
> +.SH AUTHORS
> +.UR qais.yousef@arm.com
> +Qais Yousef
> +.UE
> +.SH SEE ALSO
> +.BR nice (1),
> +.BR renice (1),
> +.BR taskset (1),
> +.BR sched (7)
> +.sp
> +See
> +.BR sched_setscheduler (2)
> +for a description of the Linux scheduling scheme.
> +.SH AVAILABILITY
> +The uclampset command is part of the util-linux package and is available from
> +https://www.kernel.org/pub/linux/utils/util-linux/.
> --
> 2.25.1
>

Re: [PATCH 1/5] Move sched_attr struct and syscall definitions into header file

[Qais Yousef]

On 01/20/21 11:25, Karel Zak wrote:
> On Sat, Jan 16, 2021 at 07:09:36PM +0000, Qais Yousef wrote:
> > So that we can re-use them in other files.
> 
> Good idea.
> 
> > +++ b/include/sched_attr.h
> > @@ -0,0 +1,97 @@
> > +/*
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License, version 2, as
> > + * published by the Free Software Foundation
> 
> My goal is to keep include/ and lib/ usable for LGPL and BRD code,
> because util-linux is not only GPL code. The ideal solution is to 
> use public domain or LGPL for this shared directories. 
> 
> Maybe we can use Public Domain also for this new header file as it
> contains only ifdef fallbacks. 
> 
> The other solution is to keep GPL sched_attr.h in schedutils/ directory.

Sorry I didn't realize the licensing constraint.

Don't we need Robert's Love permission to change the license? Making it Public
Domain makes sense. There's nothing special in here.

Thanks

--
Qais Yousef

Re: [PATCH 2/5] Add uclampset schedutil

[Qais Yousef]

On 01/20/21 13:21, Karel Zak wrote:
> 
>  Hi,
> 
> pretty readable code (thanks), I have some notes:

Thanks!

> 
> On Sat, Jan 16, 2021 at 07:09:37PM +0000, Qais Yousef wrote:
> >  #if defined (__linux__)
> >  # include <sys/syscall.h>
> >  #endif
> > @@ -73,6 +99,10 @@ struct sched_attr {
> >  	uint64_t sched_runtime;
> >  	uint64_t sched_deadline;
> >  	uint64_t sched_period;
> > +
> > +	/* UTILIZATION CLAMPING */
> > +	uint32_t sched_util_min;
> > +	uint32_t sched_util_max;
> >  };
> 
> This sched_attr definition is used only if regular system include files
> do not contain struct sched_attr (aka ifndef HAVE_SCHED_SETATTR).
> 
> What is expected on systems with header files from Linux < 5.3 where
> is sched_attr, but without sched_util_{min,max}?

Yes we will have a compilation error as you mention below.

> 
> I guess you need to check in ./configure.ac that struct sched_attr is
> usable for your uclampset otherwise you will see compilation error.
> You can use for example AC_LINK_IFELSE([AC_LANG_PROGRAM([[ ... and
> try to compile code with sched_attr.sched_util_min = 0.
> 
> I see two possible ways:
> 
> - do not compile uclampset at all if there is no sched_attr.sched_util_*
>   in system header files
> 
> - use you local definition from sched_attr.h, but it will probably
>   require to define own struct to avoid names collision
> 
> IMHO the first solution is more simple and it will keep your code readable.
> 
> If you not sure with the build-system stuff then ignore it and don't
> waste time with it. I can fix it before merge into master branch.

I will try. If it proved to be taking me time to get this done, I'll leave it
to you then. Thanks for offering to fix this up!

> 
> > +#define PROCFS(file)	"/proc/sys/kernel/" #file
> > +
> > +#define PROCFS_UCLAMP_MIN	PROCFS(sched_util_clamp_min)
> > +#define PROCFS_UCLAMP_MAX	PROCFS(sched_util_clamp_max)
> 
> Please, add it to include/pathnames.h as
>  
>  #define _PATH_PROC_KERNEL      "/proc/sys/kernel"
>  #define _PATH_PROC_UCLAMP_MIN  PATH_PROC_KERNEL "/sched_util_clamp_min"
>  #defene _PATH_PROC_UCLAMP_MAN  PATH_PROC_KERNEL "/sched_util_clamp_max"

Fixed.

> 
> 
> > +#define MAX_OPT		1000
> > +#define COMM_LEN	64
> > +#define NOT_SET		-1U
> > +
> > +struct uclampset {
> > +	unsigned int util_min;
> > +	unsigned int util_max;
> > +
> > +	pid_t pid;
> > +	unsigned int all_tasks;			/* all threads of the PID */
> > +
> > +	unsigned int system;
> > +
> > +	unsigned int verbose;
> 
> Seems like we can use bits only:
> 
>     unsigned int all_tasks:1
>                  system:1,
>                  verbose:1;

Fixed.

> 
> > +static void __attribute__((__noreturn__)) usage(void)
> > +{
> > +	FILE *out = stdout;
> > +
> > +	fputs(_("Show or change the utilization clamping attributes of a process or the system.\n"), out);
> 
> We usually have this description after synopsis and before "Options".

Fixed.

> 
> > +	fputs(USAGE_SEPARATOR, out);
> > +	fputs(_("Set util clamp for a process:\n"
> > +	" uclampset [options] [-m <util_min>] [-M <util_max>] [cmd <arg>...]\n"
> > +	" uclampset [options] [-m <util_min>] [-M <util_max>] --pid <pid>\n"), out);
> > +	fputs(USAGE_SEPARATOR, out);
> > +	fputs(_("Get util clamp for a process:\n"
> > +	" uclampset [options] -p <pid>\n"), out);
> > +
> > +	fputs(USAGE_SEPARATOR, out);
> > +	fputs(_("Set util clamp for the sytem:\n"
> > +	" uclampset [options] --system [-m <util_min>] [-M <util_max>]\n"), out);
> > +	fputs(USAGE_SEPARATOR, out);
> > +	fputs(_("Get util clamp for the system:\n"
> > +	" uclampset [options] -s\n"), out);
> 
> You do not have to be so verbose :-)
> 
>         fprintf(out,
>               _(" %1$s [options]\n"
>                 " %1$s [options] --pid <pid> | --system | <command> <arg>...\n",
>                 program_invocation_short_name);
> seems enough.

:-)

I got this now

	$ ./uclampset -h
	 uclampset [options]
	 uclampset [options] --pid <pid> | --system | <command> <arg>...

	Show or change the utilization clamping attributes of a process or the system.
	Utilization range: [0:1024]

	Options:
	 -m                   util_min value to set
	 -M                   util_max value to set
	 -a, --all-tasks      operate on all the tasks (threads) for a given pid
	 -p, --pid            operate on existing given pid
	 -s, --system         operate on system
	 -v, --verbose        display status information

	 -h, --help           display this help
	 -V, --version        display version

	For more details, see uclampset(1).

> 
> > +	fputs(USAGE_SEPARATOR, out);
> > +	fputs(_("Other options:\n"), out);
> 
> fputs(USAGE_OPTIONS, out);

Fixed.

> 
> > +	fputs(_(" -m                   util_min value to set\n"), out);
> > +	fputs(_(" -M                   util_max value to set\n"), out);
> > +	fputs(_(" -a, --all-tasks      operate on all the tasks (threads) for a given pid\n"), out);
> > +	fputs(_(" -p, --pid            operate on existing given pid\n"), out);
> > +	fputs(_(" -s, --system         operate on system\n"), out);
> > +	fputs(_(" --max                show min and max valid uclamp values\n"), out);
> > +	fputs(_(" -v, --verbose        display status information\n"), out);
> > +
> > +	fputs(USAGE_SEPARATOR, out);
> > +	printf(USAGE_HELP_OPTIONS(22));
> > +
> > +	printf(USAGE_MAN_TAIL("uclampset(1)"));
> > +	exit(EXIT_SUCCESS);
> > +}
> > +
> > +static void proc_pid_name(pid_t pid, char *name, int len)
> > +{
> > +	char *proc_comm_fmt = "/proc/%d/comm";
> > +	char proc_comm[COMM_LEN];
> > +	FILE *fp;
> > +	int size;
> > +
> > +	size = snprintf(proc_comm, COMM_LEN, proc_comm_fmt, pid);
> > +	if (size >= COMM_LEN || size < 0)
> > +		goto error;
> > +
> > +	fp = fopen(proc_comm, "r");
> > +	if (!fp)
> > +		goto error;
> > +
> > +	size = fread(name, 1, len, fp);
> > +	name[size-1] = '\0';
> > +
> > +	fclose(fp);
> > +
> > +	if (ferror(fp))
> > +		goto error;
> > +
> > +	return;
> > +error:
> > +	strncpy(name, "unknown", len);
> > +}
> 
> Please, use proc_get_command_name() from lib/procutils.c�

Oh, I missed this one. I did search for it but clearly failed. Fixed.

> 
> > +static void show_uclamp_pid_info(pid_t pid)
> > +{
> > +#ifdef HAVE_SCHED_SETATTR
> 
> This is unnecessary #ifdef, we will not compile uclampset at all if
> sched_getattr() and sched_setattr() are not available.

Fixed.

> 
> > +	struct sched_attr sa;
> > +	char comm[COMM_LEN];
> > +
> > +	/* don't display "pid 0" as that is confusing */
> > +	if (!pid)
> > +		pid = getpid();
> > +
> > +
> > +	proc_pid_name(pid, comm, COMM_LEN);
> > +
> > +	if (sched_getattr(pid, &sa, sizeof(sa), 0) != 0)
> > +		err(EXIT_FAILURE, _("failed to get pid %d's uclamp values"), pid);
> > +
> > +	printf(_("%s-%d\n\tutil_min: %d\n\tutil_max: %d\n"),
> > +		  comm, pid, sa.sched_util_min, sa.sched_util_max);
> 
>   comm =  proc_get_command_name(pid);
> 
>   printf(_("%s-%d\n\tutil_min: %d\n\tutil_max: %d\n"),
>           comm ? : "uknown", pid, sa.sched_util_min, sa.sched_util_max);
>   
>   free(comm);
> 
> I like the idea with command name, it seems more user-friendly
> than chrt-way:  "pid 876909's current  ...", but I'm not sure with
> multiple lines for each PID (%s-%d\n\t)
> 
> Let's imagine you will use
> 
>   uclamp --all-tasks --pid 123
> 
> in your script. It would be probably better to use
> 
>    "%s (%d) utilization: min: %d, max: %d\n"
> 
> to keep it easy to parse.

I think I had it all in one line but then found my eyes were struggling to
parse it. But I think I was being picky. Making it more machine readable will
be more useful.

Changed it as you suggested except s/utilization/util_clamp/ which I think is
more descriptive.

> 
> > +#else
> > +	err(EXIT_FAILURE, _("uclamp is not supported on this system"));
> > +#endif
> > +}
> > +
> > +static unsigned int read_uclamp_sysfs(char *dir)
> 
> Please, s/dir/filename/

Fixed.

> 
> > +{
> > +	unsigned int size;
> > +	char buf[16];
> > +	FILE *fp;
> > +
> > +	fp = fopen(dir, "r");
> > +	if (!fp)
> > +		err(EXIT_FAILURE, _("cannot open %s"), dir);
> > +
> > +	size = fread(buf, 1, sizeof(buf), fp);
> > +	buf[size-1] = '\0';
> > +
> > +	if (ferror(fp)) {
> > +		fclose(fp);
> > +		err(EXIT_FAILURE, _("error writing %s"), dir);
> 
>  s/error writing %s/cannot read %s/
> 
> > +	}
> > +
> > +	fclose(fp);
> > +
> > +	return strtou32_or_err(buf, _("invalid util clamp value"));
> 
> you can replace all this code with stuff from lib/path.c
> 
>   unsigned int res;
> 
>   if (ul_path_read_u32(NULL, &res, filename) != 0)
>     err(EXIT_FAILURE, _("cannot read %s), filename);
> 
> > +}
> > +
> > +static void write_uclamp_sysfs(char *dir, unsigned int val)
> > +{
> > +	unsigned int size;
> > +	char buf[16];
> > +	FILE *fp;
> > +
> > +	fp = fopen(dir, "w");
> > +	if (!fp)
> > +		err(EXIT_FAILURE, _("cannot open %s"), dir);
> > +
> > +	size = snprintf(buf, sizeof(buf), "%d", val);
> > +	buf[size] = '\n';
> > +	buf[size+1] = '\0';
> > +	fwrite(buf, 1, sizeof(buf), fp);
> > +
> > +	if (ferror(fp)) {
> > +		fclose(fp);
> > +		err(EXIT_FAILURE, _("error writing %s"), dir);
> 
> "cannot write %s" 
> 
> If not sure see po/util-linux.pot and use the most common string. It
> will save translators' time ;-)

Ah, handy!

> 
> > +	}
> > +
> > +	fclose(fp);
> > +}
> 
> but, you want to use:  ul_path_write_u64(NULL, val, filename)

Changed to use path.h helper functions. Missed those too. I should have tried
harder :)

> 
> > +static void show_uclamp_system_info(void)
> > +{
> > +	unsigned int min, max;
> > +
> > +	min = read_uclamp_sysfs(PROCFS_UCLAMP_MIN);
> > +	max = read_uclamp_sysfs(PROCFS_UCLAMP_MAX);
> > +
> > +	printf(_("System\n\tutil_min: %u\n\tutil_max: %u\n"), min, max);
> > +}
> > +
> > +static void show_uclamp_info(struct uclampset *ctl)
> > +{
> > +	if (ctl->system) {
> > +		show_uclamp_system_info();
> > +	} else if (ctl->all_tasks) {
> > +		pid_t tid;
> > +		struct proc_tasks *ts = proc_open_tasks(ctl->pid);
> > +
> > +		if (!ts)
> > +			err(EXIT_FAILURE, _("cannot obtain the list of tasks"));
> > +
> > +		while (!proc_next_tid(ts, &tid))
> > +			show_uclamp_pid_info(tid);
> > +
> > +		proc_close_tasks(ts);
> > +	} else {
> > +		show_uclamp_pid_info(ctl->pid);
> > +	}
> > +}
> > +
> > +static void show_min_max(void)
> > +{
> > +	printf(_("util_min and util_max must be in the range of [0:1024] inclusive\n"));
> > +}
> 
> Why we need --max to get hardcode range? Would be enough to add this
> note to --usage output?

Sounds better, yes. Done.

> 
> > +#ifndef HAVE_SCHED_SETATTR
> > +static int set_uclamp_one(struct uclampset *ctl, pid_t pid)
> > +{
> > +	err(EXIT_FAILURE, _("uclamp is not supported on this system"));
> > +}
> 
> Again, unnecessary #ifndef.

Fixed.

> 
> > +#else /* !HAVE_SCHED_SETATTR */
> > +static int set_uclamp_one(struct uclampset *ctl, pid_t pid)
> > +{
> > +	struct sched_attr sa;
> > +
> > +	if (sched_getattr(pid, &sa, sizeof(sa), 0) != 0)
> > +		err(EXIT_FAILURE, _("failed to get pid %d's uclamp values"), pid);
> > +
> > +	if (ctl->util_min != NOT_SET)
> > +		sa.sched_util_min = ctl->util_min;
> > +	if (ctl->util_max != NOT_SET)
> > +		sa.sched_util_max = ctl->util_max;
> > +
> > +	sa.sched_flags = SCHED_FLAG_KEEP_POLICY |
> > +			 SCHED_FLAG_KEEP_PARAMS |
> > +			 SCHED_FLAG_UTIL_CLAMP_MIN |
> > +			 SCHED_FLAG_UTIL_CLAMP_MAX;
> > +
> > +	return sched_setattr(pid, &sa, 0);
> > +}
> 
> What about SCHED_FLAG_RESET_ON_FORK flag (aka -R for chrt(8))? Does it
> matter for the task utilization attributes?

It should have a similar effect of resetting uclamp values on fork. I guess it
would be useful to add this here too.

There's also a special value '-1' which resets the min/max to system default.
Peter suggested (offline) I add it. I was going to use -r/-R for resetting
min/max respectively.

I will get all these reset options sorted too.

Many thanks for the comprehensive review and useful suggestions!

Cheers

--
Qais Yousef

> 
> > +#endif /* HAVE_SCHED_SETATTR */
> > +
> > +static void set_uclamp_pid(struct uclampset *ctl)
> > +{
> > +	if (ctl->all_tasks) {
> > +		pid_t tid;
> > +		struct proc_tasks *ts = proc_open_tasks(ctl->pid);
> > +
> > +		if (!ts)
> > +			err(EXIT_FAILURE, _("cannot obtain the list of tasks"));
> > +
> > +		while (!proc_next_tid(ts, &tid))
> > +			if (set_uclamp_one(ctl, tid) == -1)
> > +				err(EXIT_FAILURE, _("failed to set tid %d's uclamp values"), tid);
> > +
> > +		proc_close_tasks(ts);
> > +
> > +	} else if (set_uclamp_one(ctl, ctl->pid) == -1) {
> > +		err(EXIT_FAILURE, _("failed to set pid %d's uclamp values"), ctl->pid);
> > +	}
> > +}
> > +
> > +static void set_uclamp_system(struct uclampset *ctl)
> > +{
> > +	if (ctl->util_min == NOT_SET)
> > +		ctl->util_min = read_uclamp_sysfs(PROCFS_UCLAMP_MIN);
> > +
> > +	if (ctl->util_max == NOT_SET)
> > +		ctl->util_max = read_uclamp_sysfs(PROCFS_UCLAMP_MAX);
> > +
> > +	if (ctl->util_min > ctl->util_max) {
> > +		errno = EINVAL;
> > +		err(EXIT_FAILURE, _("util_min must be <= util_max"));
> > +	}
> > +
> > +	write_uclamp_sysfs(PROCFS_UCLAMP_MIN, ctl->util_min);
> > +	write_uclamp_sysfs(PROCFS_UCLAMP_MAX, ctl->util_max);
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +	struct uclampset _ctl = {
> > +		.pid = -1,
> > +		.util_min = NOT_SET,
> > +		.util_max = NOT_SET
> > +	};
> > +	struct uclampset *ctl = &_ctl;
> > +	bool no_input;
> > +	int c;
> > +
> > +	static const struct option longopts[] = {
> > +		{ "all-tasks",  no_argument, NULL, 'a' },
> > +		{ "pid",	no_argument, NULL, 'p' },
> > +		{ "system",	no_argument, NULL, 's' },
> > +		{ "help",	no_argument, NULL, 'h' },
> > +		{ "max",        no_argument, NULL, MAX_OPT },
> > +		{ "verbose",	no_argument, NULL, 'v' },
> > +		{ "version",	no_argument, NULL, 'V' },
> > +		{ NULL,		no_argument, NULL, 0 }
> > +	};
> > +
> > +	setlocale(LC_ALL, "");
> > +	bindtextdomain(PACKAGE, LOCALEDIR);
> > +	textdomain(PACKAGE);
> > +	close_stdout_atexit();
> > +
> > +	while((c = getopt_long(argc, argv, "+asphmMvV", longopts, NULL)) != -1)
> > +	{
> > +		switch (c) {
> > +		case 'a':
> > +			ctl->all_tasks = 1;
> > +			break;
> > +		case MAX_OPT:
> > +			show_min_max();
> > +			return EXIT_SUCCESS;
> > +		case 'p':
> > +			errno = 0;
> > +			ctl->pid = strtos32_or_err(argv[optind], _("invalid PID argument"));
> > +			optind++;
> > +			break;
> > +		case 's':
> > +			ctl->system = 1;
> > +			break;
> > +		case 'v':
> > +			ctl->verbose = 1;
> > +			break;
> > +		case 'm':
> > +			ctl->util_min = strtos32_or_err(argv[optind], _("invalid util_min argument"));
> > +			optind++;
> > +			break;
> > +		case 'M':
> > +			ctl->util_max = strtos32_or_err(argv[optind], _("invalid util_max argument"));
> > +			optind++;
> > +			break;
> > +		case 'V':
> > +			print_version(EXIT_SUCCESS);
> > +			/* fallthrough */
> > +		case 'h':
> > +			usage();
> > +		default:
> > +			errtryhelp(EXIT_FAILURE);
> > +		}
> > +	}
> > +
> > +	no_input = ctl->util_min == NOT_SET && ctl->util_max == NOT_SET;
> > +
> > +	if (no_input) {
> > +		show_uclamp_info(ctl);
> > +		return EXIT_SUCCESS;
> > +	}
> > +
> > +	if (ctl->pid == -1)
> > +		ctl->pid = 0;
> > +
> > +	if (ctl->system)
> > +		set_uclamp_system(ctl);
> > +	else
> > +		set_uclamp_pid(ctl);
> > +
> > +	if (ctl->verbose)
> > +		show_uclamp_info(ctl);
> > +
> > +	if (!ctl->pid && !ctl->system) {
> > +		argv += optind;
> > +		execvp(argv[0], argv);
> > +		errexec(argv[0]);
> > +	}
> > +
> > +	return EXIT_SUCCESS;
> > +}
> 
> Thanks!
> 
>  Karel
> 
> -- 
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com
> 

Re: [PATCH 3/5] uclampset: Add man page

[Qais Yousef]

On 01/20/21 15:46, Vincent Guittot wrote:
> > +.TP
> > +For example:
> > +.B uclampset \-s
> > +.I -m\ 512
> > +.I -M\ 700
> > +.PP
> > +will prevent any task from being boosted higher than 512. And all tasks in the
> 
> will prevent any task from being boosted higher than 700

700 or 512 you mean here?

> or
> will ensure  any task to be boosted higher than 512

It is certainly not that. That's the confusion I was trying to clear up in this
note. The system values are _constraints_. It sets an upper bound on the
maximum uclamp_min/max value the system will apply.

For example if a task::uclamp_min=1024 but the system::uclamp_min=512 512, then
effectively the task will only be boosted to 512. Once this restriction is left
by setting system::uclamp_min=1024 again, then the task will get the boost
value it requested.

I'm open to suggestions on how to better explain this..

Thanks

--
Qais Yousef

Re: [PATCH 3/5] uclampset: Add man page

[Qais Yousef]

On 01/20/21 13:22, Karel Zak wrote:
> On Sat, Jan 16, 2021 at 07:09:38PM +0000, Qais Yousef wrote:
> > +See
> > +.BR sched_setscheduler (2)
> > +for a description of the Linux scheduling scheme.
> 
> and maybe sched_setattr(2) too.

Yep. Done.

Thanks

--
Qais Yousef

Re: [PATCH 3/5] uclampset: Add man page

[Vincent Guittot]

On Sat, 23 Jan 2021 at 19:31, Qais Yousef <qais.yousef@arm.com> wrote:
>
> On 01/20/21 15:46, Vincent Guittot wrote:
> > > +.TP
> > > +For example:
> > > +.B uclampset \-s
> > > +.I -m\ 512
> > > +.I -M\ 700
> > > +.PP
> > > +will prevent any task from being boosted higher than 512. And all tasks in the
> >
> > will prevent any task from being boosted higher than 700
>
> 700 or 512 you mean here?
>
> > or
> > will ensure  any task to be boosted higher than 512
>
> It is certainly not that. That's the confusion I was trying to clear up in this
> note. The system values are _constraints_. It sets an upper bound on the
> maximum uclamp_min/max value the system will apply.
>
> For example if a task::uclamp_min=1024 but the system::uclamp_min=512 512, then
> effectively the task will only be boosted to 512. Once this restriction is left
> by setting system::uclamp_min=1024 again, then the task will get the boost
> value it requested.

ok, I got confused because uclamp -p PID  -min 512 - M 700 and uclamp
-s  -min 512 - M 700 look quite similar but their behaviors are quite
different.

I mean

uclamp -p PID -min 512 - M 700 will ensure that the task's utilization
will be always reported in the range [512:700]
The behavior is the same for the cgroup  with cpu.uclamp.min and cpu.uclamp.max

whereas

uclamp -s -min 512 - M 700 will do the opposite; i.e. the min of
task's utilization range will never be above 512

TBH I don't know how to make it clear that the behavior of -min is the
opposite between -p and -s

>
> I'm open to suggestions on how to better explain this..
>
> Thanks
>
> --
> Qais Yousef

Re: [PATCH 3/5] uclampset: Add man page

[Qais Yousef]

On 01/25/21 09:26, Vincent Guittot wrote:
> On Sat, 23 Jan 2021 at 19:31, Qais Yousef <qais.yousef@arm.com> wrote:
> >
> > On 01/20/21 15:46, Vincent Guittot wrote:
> > > > +.TP
> > > > +For example:
> > > > +.B uclampset \-s
> > > > +.I -m\ 512
> > > > +.I -M\ 700
> > > > +.PP
> > > > +will prevent any task from being boosted higher than 512. And all tasks in the
> > >
> > > will prevent any task from being boosted higher than 700
> >
> > 700 or 512 you mean here?
> >
> > > or
> > > will ensure  any task to be boosted higher than 512
> >
> > It is certainly not that. That's the confusion I was trying to clear up in this
> > note. The system values are _constraints_. It sets an upper bound on the
> > maximum uclamp_min/max value the system will apply.
> >
> > For example if a task::uclamp_min=1024 but the system::uclamp_min=512 512, then
> > effectively the task will only be boosted to 512. Once this restriction is left
> > by setting system::uclamp_min=1024 again, then the task will get the boost
> > value it requested.
> 
> ok, I got confused because uclamp -p PID  -min 512 - M 700 and uclamp
> -s  -min 512 - M 700 look quite similar but their behaviors are quite
> different.
> 
> I mean
> 
> uclamp -p PID -min 512 - M 700 will ensure that the task's utilization
> will be always reported in the range [512:700]
> The behavior is the same for the cgroup  with cpu.uclamp.min and cpu.uclamp.max

The cgroup behavior actually works like system. It is a constraint of your max
allowed util_min/max value. There's an exception to that where cgroup will
behave like '-p'...

I avoided commenting on cgroup behavior here. I plan to send a patch to
document util clamp in the kernel. For this tool since we don't interact with
cgroup I omitted talking about it.

I intend to send a patch documenting util clamp in the kernel. I think It's
better to discuss this details there?

> 
> whereas
> 
> uclamp -s -min 512 - M 700 will do the opposite; i.e. the min of
> task's utilization range will never be above 512
> 
> TBH I don't know how to make it clear that the behavior of -min is the
> opposite between -p and -s

Hopefully it'll become common knowledge at some point..

Thanks for having a look.

Cheers

--
Qais Yousef