util-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Add a new uclampset utility
@ 2021-01-30 20:50 Qais Yousef
  2021-01-30 20:50 ` [PATCH v2 1/5] Move sched_attr struct and syscall definitions into header file Qais Yousef
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Qais Yousef @ 2021-01-30 20:50 UTC (permalink / raw)
  To: util-linux
  Cc: Karel Zak, Bernhard Voelker, Peter Zijlstra (Intel),
	Vincent Guittot, Dietmar Eggemann, Patrick Bellasi,
	Vincent Donnefort, Qais Yousef

Changes in v2 (thanks for all reviewers!):

	* Better error handling of invalid options.
	* Expose the special -1 value to reset to system default.
	* Add new --reset-on-fork option.
	* Move sched_attr.h out of include directory to avoid licensing issues.
	* Use pathnames.h for new procfs defines.
	* Use common functions to get command name and handle procfs
	  read/write.
	* Remove --max option and document in help message.
	* Improve help message formatting.
	* Minor manpage tweaks.

A branch of the changes is available at

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

Karel, I compiled this v2 on an old Ubuntu 18.04 machine running v4.15 kernel
and didn't see any compilation errors. chrt worked fine and uclampset returns
EINVAL errors as expected. I don't think we need to do anything to deal with
compiling against old kernel headers.


Orignal cover letter
====================

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/pathnames.h           |   4 +
 schedutils/Makemodule.am      |   7 +
 schedutils/chrt.c             |  77 +-------
 schedutils/sched_attr.h       | 127 +++++++++++++
 schedutils/uclampset.1        | 174 ++++++++++++++++++
 schedutils/uclampset.c        | 337 ++++++++++++++++++++++++++++++++++
 10 files changed, 702 insertions(+), 76 deletions(-)
 create mode 100644 bash-completion/uclampset
 create mode 100644 schedutils/sched_attr.h
 create mode 100644 schedutils/uclampset.1
 create mode 100644 schedutils/uclampset.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/5] Move sched_attr struct and syscall definitions into header file
  2021-01-30 20:50 [PATCH v2 0/5] Add a new uclampset utility Qais Yousef
@ 2021-01-30 20:50 ` Qais Yousef
  2021-01-30 20:50 ` [PATCH v2 2/5] Add uclampset schedutil Qais Yousef
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Qais Yousef @ 2021-01-30 20:50 UTC (permalink / raw)
  To: util-linux
  Cc: Karel Zak, Bernhard Voelker, Peter Zijlstra (Intel),
	Vincent Guittot, Dietmar Eggemann, Patrick Bellasi,
	Vincent Donnefort, Qais Yousef

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

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
---
 schedutils/chrt.c       | 77 +-------------------------------
 schedutils/sched_attr.h | 97 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 76 deletions(-)
 create mode 100644 schedutils/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 {
diff --git a/schedutils/sched_attr.h b/schedutils/sched_attr.h
new file mode 100644
index 000000000..b39d37b5d
--- /dev/null
+++ b/schedutils/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 */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/5] Add uclampset schedutil
  2021-01-30 20:50 [PATCH v2 0/5] Add a new uclampset utility Qais Yousef
  2021-01-30 20:50 ` [PATCH v2 1/5] Move sched_attr struct and syscall definitions into header file Qais Yousef
@ 2021-01-30 20:50 ` Qais Yousef
  2021-02-01 16:12   ` Karel Zak
  2021-01-30 20:50 ` [PATCH v2 3/5] uclampset: Add man page Qais Yousef
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Qais Yousef @ 2021-01-30 20:50 UTC (permalink / raw)
  To: util-linux
  Cc: Karel Zak, Bernhard Voelker, Peter Zijlstra (Intel),
	Vincent Guittot, Dietmar Eggemann, Patrick Bellasi,
	Vincent Donnefort, 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/pathnames.h     |   4 +
 schedutils/sched_attr.h |  30 ++++
 schedutils/uclampset.c  | 337 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 372 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/pathnames.h b/include/pathnames.h
index 8f6233749..69a0b5524 100644
--- a/include/pathnames.h
+++ b/include/pathnames.h
@@ -207,4 +207,8 @@
 #define _PATH_DEV_RFKILL	"/dev/rfkill"
 #define _PATH_SYS_RFKILL	"/sys/class/rfkill"
 
+#define _PATH_PROC_KERNEL(file)	"/proc/sys/kernel" #file
+#define _PATH_PROC_UCLAMP_MIN	_PATH_PROC_KERNEL(/sched_util_clamp_min)
+#define _PATH_PROC_UCLAMP_MAX	_PATH_PROC_KERNEL(/sched_util_clamp_max)
+
 #endif /* PATHNAMES_H */
diff --git a/schedutils/sched_attr.h b/schedutils/sched_attr.h
index b39d37b5d..39d8532a3 100644
--- a/schedutils/sched_attr.h
+++ b/schedutils/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-2021 Qais Yousef
+ * Copyright (C) 2020-2021 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..b9a02aae1
--- /dev/null
+++ b/schedutils/uclampset.c
@@ -0,0 +1,337 @@
+/*
+ * 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-2021 Qais Yousef
+ * Copyright (C) 2020-2021 Arm Ltd
+ */
+
+#include <errno.h>
+#include <getopt.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "closestream.h"
+#include "path.h"
+#include "pathnames.h"
+#include "procutils.h"
+#include "sched_attr.h"
+#include "strutils.h"
+
+#define NOT_SET		-2U
+
+struct uclampset {
+	unsigned int util_min;
+	unsigned int util_max;
+
+	pid_t pid;
+	unsigned int	all_tasks:1,		/* all threads of the PID */
+			system:1,
+			util_min_set:1,		/* indicates -m option was passed */
+			util_max_set:1,		/* indicates -M option was passed */
+			reset_on_fork:1,
+			verbose:1;
+	char *cmd;
+};
+
+static void __attribute__((__noreturn__)) usage(void)
+{
+	FILE *out = stdout;
+
+	fprintf(out,
+		_(" %1$s [options]\n"
+		  " %1$s [options] --pid <pid> | --system | <command> <arg>...\n"),
+		program_invocation_short_name);
+
+	fputs(USAGE_SEPARATOR, out);
+	fputs(_("Show or change the utilization clamping attributes of a process or the system.\n"), out);
+	fputs(_("Utilization range: [0:1024]\n"), out);
+	fputs(_("Use special -1 value to reset to system's default\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(_(" -R, --reset-on-fork  set reset-on-fork flag\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 show_uclamp_pid_info(pid_t pid, char *cmd)
+{
+	struct sched_attr sa;
+	char *comm;
+
+	/* don't display "pid 0" as that is confusing */
+	if (!pid)
+		pid = getpid();
+
+	if (sched_getattr(pid, &sa, sizeof(sa), 0) != 0)
+		err(EXIT_FAILURE, _("failed to get pid %d's uclamp values"), pid);
+
+	if (cmd)
+		comm = cmd;
+	else
+		comm = proc_get_command_name(pid);
+
+	printf(_("%s (%d) util_clamp: min: %d max: %d\n"),
+	       comm ? : "uknown", pid, sa.sched_util_min, sa.sched_util_max);
+
+	if (!cmd)
+		free(comm);
+}
+
+static unsigned int read_uclamp_sysfs(char *filename)
+{
+	unsigned int val;
+
+	if (ul_path_read_u32(NULL, &val, filename) != 0)
+		err(EXIT_FAILURE, _("cannot read %s"), filename);
+
+	return val;
+}
+
+static void write_uclamp_sysfs(char *filename, unsigned int val)
+{
+	if (ul_path_write_u64(NULL, val, filename) != 0)
+		err(EXIT_FAILURE, _("cannot write %s"), filename);
+}
+
+static void show_uclamp_system_info(void)
+{
+	unsigned int min, max;
+
+	min = read_uclamp_sysfs(_PATH_PROC_UCLAMP_MIN);
+	max = read_uclamp_sysfs(_PATH_PROC_UCLAMP_MAX);
+
+	printf(_("System util_clamp: min: %u 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, NULL);
+
+		proc_close_tasks(ts);
+	} else {
+		show_uclamp_pid_info(ctl->pid, ctl->cmd);
+	}
+}
+
+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_set)
+		sa.sched_util_min = ctl->util_min;
+	if (ctl->util_max_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;
+
+	if (ctl->reset_on_fork)
+		sa.sched_flags |= SCHED_FLAG_RESET_ON_FORK;
+
+	return sched_setattr(pid, &sa, 0);
+}
+
+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_set)
+		ctl->util_min = read_uclamp_sysfs(_PATH_PROC_UCLAMP_MIN);
+
+	if (!ctl->util_max_set)
+		ctl->util_max = read_uclamp_sysfs(_PATH_PROC_UCLAMP_MAX);
+
+	if (ctl->util_min > ctl->util_max) {
+		errno = EINVAL;
+		err(EXIT_FAILURE, _("util_min must be <= util_max"));
+	}
+
+	write_uclamp_sysfs(_PATH_PROC_UCLAMP_MIN, ctl->util_min);
+	write_uclamp_sysfs(_PATH_PROC_UCLAMP_MAX, ctl->util_max);
+}
+
+static void validate_util(int val)
+{
+	if (val > 1024 || val < -1) {
+		errno = EINVAL;
+		err(EXIT_FAILURE, _("%d out of range"), val);
+	}
+}
+
+int main(int argc, char **argv)
+{
+	struct uclampset _ctl = {
+		.pid = -1,
+		.util_min = NOT_SET,
+		.util_max = NOT_SET,
+		.cmd = NULL
+	};
+	struct uclampset *ctl = &_ctl;
+	int c;
+
+	static const struct option longopts[] = {
+		{ "all-tasks",		no_argument, NULL, 'a' },
+		{ "pid",		no_argument, NULL, 'p' },
+		{ "system",		no_argument, NULL, 's' },
+		{ "reset-on-fork",	no_argument, NULL, 'R' },
+		{ "help",		no_argument, NULL, 'h' },
+		{ "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, "+asRphmMvV", longopts, NULL)) != -1)
+	{
+		switch (c) {
+		case 'a':
+			ctl->all_tasks = 1;
+			break;
+		case 'p':
+			errno = 0;
+			ctl->pid = strtos32_or_err(argv[optind], _("invalid PID argument"));
+			optind++;
+			break;
+		case 's':
+			ctl->system = 1;
+			break;
+		case 'R':
+			ctl->reset_on_fork = 1;
+			break;
+		case 'v':
+			ctl->verbose = 1;
+			break;
+		case 'm':
+			ctl->util_min = strtos32_or_err(argv[optind], _("invalid util_min argument"));
+			ctl->util_min_set = 1;
+			validate_util(ctl->util_min);
+			optind++;
+			break;
+		case 'M':
+			ctl->util_max = strtos32_or_err(argv[optind], _("invalid util_max argument"));
+			ctl->util_max_set = 1;
+			validate_util(ctl->util_max);
+			optind++;
+			break;
+		case 'V':
+			print_version(EXIT_SUCCESS);
+			/* fallthrough */
+		case 'h':
+			usage();
+		default:
+			errtryhelp(EXIT_FAILURE);
+		}
+	}
+
+	if (argc == 1) {
+		usage();
+		exit(EXIT_FAILURE);
+	}
+
+	/* all_tasks implies --pid */
+	if (ctl->all_tasks && ctl->pid == -1) {
+		errno = EINVAL;
+		err(EXIT_FAILURE, _("missing -p option"));
+	}
+
+	if (!ctl->util_min_set && !ctl->util_max_set) {
+		/* -p or -s must be passed */
+		if (!ctl->system && ctl->pid == -1) {
+			usage();
+			exit(EXIT_FAILURE);
+		}
+
+		show_uclamp_info(ctl);
+		return EXIT_SUCCESS;
+	}
+
+	/* ensure there's a command to execute if no -s or -p */
+	if (!ctl->system && ctl->pid == -1) {
+		if (argc <= optind) {
+			errno = EINVAL;
+			err(EXIT_FAILURE, _("no cmd to execute"));
+		}
+
+		argv += optind;
+		ctl->cmd = argv[0];
+	}
+
+	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->cmd) {
+		execvp(ctl->cmd, argv);
+		errexec(ctl->cmd);
+	}
+
+	return EXIT_SUCCESS;
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/5] uclampset: Add man page
  2021-01-30 20:50 [PATCH v2 0/5] Add a new uclampset utility Qais Yousef
  2021-01-30 20:50 ` [PATCH v2 1/5] Move sched_attr struct and syscall definitions into header file Qais Yousef
  2021-01-30 20:50 ` [PATCH v2 2/5] Add uclampset schedutil Qais Yousef
@ 2021-01-30 20:50 ` Qais Yousef
  2021-01-30 20:50 ` [PATCH v2 4/5] uclampset: Plump into the build system Qais Yousef
  2021-01-30 20:50 ` [PATCH v2 5/5] uclampset: Plumb in bash-completion Qais Yousef
  4 siblings, 0 replies; 12+ messages in thread
From: Qais Yousef @ 2021-01-30 20:50 UTC (permalink / raw)
  To: util-linux
  Cc: Karel Zak, Bernhard Voelker, Peter Zijlstra (Intel),
	Vincent Guittot, Dietmar Eggemann, Patrick Bellasi,
	Vincent Donnefort, Qais Yousef

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

diff --git a/schedutils/uclampset.1 b/schedutils/uclampset.1
new file mode 100644
index 000000000..d877ac6b9
--- /dev/null
+++ b/schedutils/uclampset.1
@@ -0,0 +1,174 @@
+.\" uclampset(1) manpage
+.\"
+.\" Copyright (C) 2020-2021 Qais Yousef <qais.yousef@arm.com>
+.\" Copyright (C) 2020-2021 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
+.B 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
+.B 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].
+The special value -1 is used to reset to system's default.
+
+.SH OPTIONS
+.TP
+.B \-m
+Set util_min value.
+.TP
+.B \-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 \-p ,\  \-\-pid
+Operate on an existing PID and do not launch a new task.
+.TP
+.BR \-s ,\  \-\-system
+Set or retrieve the system-wide utilization clamping attributes.
+.TP
+.BR \-R ,\  \-\-reset-on-fork
+Set
+.B SCHED_FLAG_RESET_ON_FORK
+flag
+.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
+.B uclamp_min
+and
+.B 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)
+and
+.BR sched_setattr (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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/5] uclampset: Plump into the build system
  2021-01-30 20:50 [PATCH v2 0/5] Add a new uclampset utility Qais Yousef
                   ` (2 preceding siblings ...)
  2021-01-30 20:50 ` [PATCH v2 3/5] uclampset: Add man page Qais Yousef
@ 2021-01-30 20:50 ` Qais Yousef
  2021-01-30 20:50 ` [PATCH v2 5/5] uclampset: Plumb in bash-completion Qais Yousef
  4 siblings, 0 replies; 12+ messages in thread
From: Qais Yousef @ 2021-01-30 20:50 UTC (permalink / raw)
  To: util-linux
  Cc: Karel Zak, Bernhard Voelker, Peter Zijlstra (Intel),
	Vincent Guittot, Dietmar Eggemann, Patrick Bellasi,
	Vincent Donnefort, 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/5] uclampset: Plumb in bash-completion
  2021-01-30 20:50 [PATCH v2 0/5] Add a new uclampset utility Qais Yousef
                   ` (3 preceding siblings ...)
  2021-01-30 20:50 ` [PATCH v2 4/5] uclampset: Plump into the build system Qais Yousef
@ 2021-01-30 20:50 ` Qais Yousef
  4 siblings, 0 replies; 12+ messages in thread
From: Qais Yousef @ 2021-01-30 20:50 UTC (permalink / raw)
  To: util-linux
  Cc: Karel Zak, Bernhard Voelker, Peter Zijlstra (Intel),
	Vincent Guittot, Dietmar Eggemann, Patrick Bellasi,
	Vincent Donnefort, 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..87b5b378f
--- /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
+				--pid
+				--system
+				--reset-on-fork
+				--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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] Add uclampset schedutil
  2021-01-30 20:50 ` [PATCH v2 2/5] Add uclampset schedutil Qais Yousef
@ 2021-02-01 16:12   ` Karel Zak
  2021-02-01 17:32     ` Qais Yousef
  0 siblings, 1 reply; 12+ messages in thread
From: Karel Zak @ 2021-02-01 16:12 UTC (permalink / raw)
  To: Qais Yousef
  Cc: util-linux, Bernhard Voelker, Peter Zijlstra (Intel),
	Vincent Guittot, Dietmar Eggemann, Patrick Bellasi,
	Vincent Donnefort

On Sat, Jan 30, 2021 at 08:50:36PM +0000, Qais Yousef wrote:

Now I see that you have reused chrt concept of --pid. We keep it for
chrt due to backward compatibility, but for new util it would be
better to use standard 'required_argument' for --pid.

> +	static const struct option longopts[] = {
> +		{ "all-tasks",		no_argument, NULL, 'a' },
> +		{ "pid",		no_argument, NULL, 'p' },

 { "pid", required_argument, NULL, 'p' },

> +		{ "system",		no_argument, NULL, 's' },
> +		{ "reset-on-fork",	no_argument, NULL, 'R' },
> +		{ "help",		no_argument, NULL, 'h' },
> +		{ "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, "+asRphmMvV", longopts, NULL)) != -1)

 The same we can do for -m and -M, just normal options where getopt_long() 
 use optarg:

   "+asRp:hm:M:vV"

> +	{
> +		switch (c) {
> +		case 'a':
> +			ctl->all_tasks = 1;
> +			break;
> +		case 'p':
> +			errno = 0;
> +			ctl->pid = strtos32_or_err(argv[optind], _("invalid PID argument"));


 ctl->pid = strtos32_or_err(optarg, _("invalid PID argument")); 

> +			optind++;
> +			break;
> +		case 's':
> +			ctl->system = 1;
> +			break;
> +		case 'R':
> +			ctl->reset_on_fork = 1;
> +			break;
> +		case 'v':
> +			ctl->verbose = 1;
> +			break;
> +		case 'm':
> +			ctl->util_min = strtos32_or_err(argv[optind], _("invalid util_min argument"));

 ctl->util_min = strtos32_or_err(optarg, _("invalid util_min argument"));
 

> +			ctl->util_min_set = 1;
> +			validate_util(ctl->util_min);
> +			optind++;
> +			break;
> +		case 'M':
> +			ctl->util_max = strtos32_or_err(argv[optind], _("invalid util_max argument"));

 ctl->util_max = strtos32_or_err(optarg, _("invalid util_max argument"));

> +			ctl->util_max_set = 1;
> +			validate_util(ctl->util_max);
> +			optind++;
> +			break;
> +		case 'V':
> +			print_version(EXIT_SUCCESS);
> +			/* fallthrough */
> +		case 'h':
> +			usage();
> +		default:
> +			errtryhelp(EXIT_FAILURE);
> +		}
> +	}


 Karel


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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] Add uclampset schedutil
  2021-02-01 16:12   ` Karel Zak
@ 2021-02-01 17:32     ` Qais Yousef
  2021-02-02  8:01       ` Karel Zak
  0 siblings, 1 reply; 12+ messages in thread
From: Qais Yousef @ 2021-02-01 17:32 UTC (permalink / raw)
  To: Karel Zak
  Cc: util-linux, Bernhard Voelker, Peter Zijlstra (Intel),
	Vincent Guittot, Dietmar Eggemann, Patrick Bellasi,
	Vincent Donnefort

On 02/01/21 17:12, Karel Zak wrote:
> On Sat, Jan 30, 2021 at 08:50:36PM +0000, Qais Yousef wrote:
> 
> Now I see that you have reused chrt concept of --pid. We keep it for
> chrt due to backward compatibility, but for new util it would be
> better to use standard 'required_argument' for --pid.

Hmm what does required mean here? --pid is optional. But if passed, a value
is required to be passed indeed.

> 
> > +	static const struct option longopts[] = {
> > +		{ "all-tasks",		no_argument, NULL, 'a' },
> > +		{ "pid",		no_argument, NULL, 'p' },
> 
>  { "pid", required_argument, NULL, 'p' },

Assuming this means --pid must be followed by a value then indeed that's what
we expect to happen.

> 
> > +		{ "system",		no_argument, NULL, 's' },
> > +		{ "reset-on-fork",	no_argument, NULL, 'R' },
> > +		{ "help",		no_argument, NULL, 'h' },
> > +		{ "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, "+asRphmMvV", longopts, NULL)) != -1)
> 
>  The same we can do for -m and -M, just normal options where getopt_long() 
>  use optarg:
> 
>    "+asRp:hm:M:vV"

+1

> 
> > +	{
> > +		switch (c) {
> > +		case 'a':
> > +			ctl->all_tasks = 1;
> > +			break;
> > +		case 'p':
> > +			errno = 0;
> > +			ctl->pid = strtos32_or_err(argv[optind], _("invalid PID argument"));
> 
> 
>  ctl->pid = strtos32_or_err(optarg, _("invalid PID argument")); 

+1

will fix this and the 2 other occurrences below to include in v3.

Thanks!

--
Qais Yousef

> 
> > +			optind++;
> > +			break;
> > +		case 's':
> > +			ctl->system = 1;
> > +			break;
> > +		case 'R':
> > +			ctl->reset_on_fork = 1;
> > +			break;
> > +		case 'v':
> > +			ctl->verbose = 1;
> > +			break;
> > +		case 'm':
> > +			ctl->util_min = strtos32_or_err(argv[optind], _("invalid util_min argument"));
> 
>  ctl->util_min = strtos32_or_err(optarg, _("invalid util_min argument"));
>  
> 
> > +			ctl->util_min_set = 1;
> > +			validate_util(ctl->util_min);
> > +			optind++;
> > +			break;
> > +		case 'M':
> > +			ctl->util_max = strtos32_or_err(argv[optind], _("invalid util_max argument"));
> 
>  ctl->util_max = strtos32_or_err(optarg, _("invalid util_max argument"));
> 
> > +			ctl->util_max_set = 1;
> > +			validate_util(ctl->util_max);
> > +			optind++;
> > +			break;
> > +		case 'V':
> > +			print_version(EXIT_SUCCESS);
> > +			/* fallthrough */
> > +		case 'h':
> > +			usage();
> > +		default:
> > +			errtryhelp(EXIT_FAILURE);
> > +		}
> > +	}
> 
> 
>  Karel
> 
> 
> -- 
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] Add uclampset schedutil
  2021-02-01 17:32     ` Qais Yousef
@ 2021-02-02  8:01       ` Karel Zak
  2021-02-02 11:02         ` Qais Yousef
  0 siblings, 1 reply; 12+ messages in thread
From: Karel Zak @ 2021-02-02  8:01 UTC (permalink / raw)
  To: Qais Yousef; +Cc: util-linux

On Mon, Feb 01, 2021 at 05:32:02PM +0000, Qais Yousef wrote:
> On 02/01/21 17:12, Karel Zak wrote:
> > On Sat, Jan 30, 2021 at 08:50:36PM +0000, Qais Yousef wrote:
> > 
> > Now I see that you have reused chrt concept of --pid. We keep it for
> > chrt due to backward compatibility, but for new util it would be
> > better to use standard 'required_argument' for --pid.
> 
> Hmm what does required mean here? --pid is optional. But if passed, a value
> is required to be passed indeed.

It means a value required if --pid specified.

> > > +	static const struct option longopts[] = {
> > > +		{ "all-tasks",		no_argument, NULL, 'a' },
> > > +		{ "pid",		no_argument, NULL, 'p' },
> > 
> >  { "pid", required_argument, NULL, 'p' },
> 
> Assuming this means --pid must be followed by a value then indeed that's what
> we expect to happen.

 Yes.

> will fix this and the 2 other occurrences below to include in v3.

You don't have to send it to the mailing list, juts update your
github repo and I'll merge it :-) Thanks.

    Karel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] Add uclampset schedutil
  2021-02-02  8:01       ` Karel Zak
@ 2021-02-02 11:02         ` Qais Yousef
  2021-02-02 15:40           ` Karel Zak
  0 siblings, 1 reply; 12+ messages in thread
From: Qais Yousef @ 2021-02-02 11:02 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On 02/02/21 09:01, Karel Zak wrote:
> On Mon, Feb 01, 2021 at 05:32:02PM +0000, Qais Yousef wrote:
> > On 02/01/21 17:12, Karel Zak wrote:
> > > On Sat, Jan 30, 2021 at 08:50:36PM +0000, Qais Yousef wrote:
> > > 
> > > Now I see that you have reused chrt concept of --pid. We keep it for
> > > chrt due to backward compatibility, but for new util it would be
> > > better to use standard 'required_argument' for --pid.
> > 
> > Hmm what does required mean here? --pid is optional. But if passed, a value
> > is required to be passed indeed.
> 
> It means a value required if --pid specified.
> 
> > > > +	static const struct option longopts[] = {
> > > > +		{ "all-tasks",		no_argument, NULL, 'a' },
> > > > +		{ "pid",		no_argument, NULL, 'p' },
> > > 
> > >  { "pid", required_argument, NULL, 'p' },
> > 
> > Assuming this means --pid must be followed by a value then indeed that's what
> > we expect to happen.
> 
>  Yes.
> 
> > will fix this and the 2 other occurrences below to include in v3.
> 
> You don't have to send it to the mailing list, juts update your
> github repo and I'll merge it :-) Thanks.

Thanks a lot Karel. Pushed a new uclampset-v3 branch so that you can easily
review the diff against v2 before merging.

Cheers

--
Qais Yousef

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] Add uclampset schedutil
  2021-02-02 11:02         ` Qais Yousef
@ 2021-02-02 15:40           ` Karel Zak
  2021-02-02 16:29             ` Qais Yousef
  0 siblings, 1 reply; 12+ messages in thread
From: Karel Zak @ 2021-02-02 15:40 UTC (permalink / raw)
  To: Qais Yousef; +Cc: util-linux

On Tue, Feb 02, 2021 at 11:02:25AM +0000, Qais Yousef wrote:
> Thanks a lot Karel. Pushed a new uclampset-v3 branch so that you can easily
> review the diff against v2 before merging.

Merged. I did some minor changes to --help and ./configure by other
commits.

Thanks.

    Karel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] Add uclampset schedutil
  2021-02-02 15:40           ` Karel Zak
@ 2021-02-02 16:29             ` Qais Yousef
  0 siblings, 0 replies; 12+ messages in thread
From: Qais Yousef @ 2021-02-02 16:29 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On 02/02/21 16:40, Karel Zak wrote:
> On Tue, Feb 02, 2021 at 11:02:25AM +0000, Qais Yousef wrote:
> > Thanks a lot Karel. Pushed a new uclampset-v3 branch so that you can easily
> > review the diff against v2 before merging.
> 
> Merged. I did some minor changes to --help and ./configure by other
> commits.

Great, thanks!

Cheers

--
Qais Yousef

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-02-02 16:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-30 20:50 [PATCH v2 0/5] Add a new uclampset utility Qais Yousef
2021-01-30 20:50 ` [PATCH v2 1/5] Move sched_attr struct and syscall definitions into header file Qais Yousef
2021-01-30 20:50 ` [PATCH v2 2/5] Add uclampset schedutil Qais Yousef
2021-02-01 16:12   ` Karel Zak
2021-02-01 17:32     ` Qais Yousef
2021-02-02  8:01       ` Karel Zak
2021-02-02 11:02         ` Qais Yousef
2021-02-02 15:40           ` Karel Zak
2021-02-02 16:29             ` Qais Yousef
2021-01-30 20:50 ` [PATCH v2 3/5] uclampset: Add man page Qais Yousef
2021-01-30 20:50 ` [PATCH v2 4/5] uclampset: Plump into the build system Qais Yousef
2021-01-30 20:50 ` [PATCH v2 5/5] uclampset: Plumb in bash-completion Qais Yousef

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).