Merge pull request #142 from mdiluz/fixes-and-longoptions

ioprio and niceness fixes, and long cmdline options

daemon/daemon_config.c

@@ -31,6 +31,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #define _GNU_SOURCE
 
 #include "daemon_config.h"
+#include "helpers.h"
 #include "logging.h"
 
 /* Ben Hoyt's inih library */
@@ -548,6 +549,11 @@ long config_get_renice_value(GameModeConfig *self)
 {
 	long value = 0;
 	memcpy_locked_config(self, &value, &self->values.renice, sizeof(long));
+	/* Validate the renice value */
+	if ((value < 1 || value > 20) && value != 0) {
+		LOG_ONCE(ERROR, "Configured renice value '%ld' is invalid, will not renice.\n", value);
+		value = 0;
+	}
 	return value;
 }
 
@@ -559,12 +565,30 @@ long config_get_ioprio_value(GameModeConfig *self)
 	long value = 0;
 	char ioprio_value[CONFIG_VALUE_MAX] = { 0 };
 	memcpy_locked_config(self, ioprio_value, &self->values.ioprio, sizeof(self->values.ioprio));
+
+	/* account for special string values */
 	if (0 == strncmp(ioprio_value, "off", sizeof(self->values.ioprio)))
 		value = IOPRIO_DONT_SET;
 	else if (0 == strncmp(ioprio_value, "default", sizeof(self->values.ioprio)))
 		value = IOPRIO_RESET_DEFAULT;
 	else
 		value = atoi(ioprio_value);
+
+	/* Validate values */
+	if (IOPRIO_RESET_DEFAULT == value) {
+		LOG_ONCE(MSG, "IO priority will be reset to default behavior (based on CPU priority).\n");
+		value = 0;
+	} else {
+		/* maybe clamp the value */
+		long invalid_ioprio = value;
+		value = CLAMP(0, 7, value);
+		if (value != invalid_ioprio)
+			LOG_ONCE(ERROR,
+			         "IO priority value %ld invalid, clamping to %ld\n",
+			         invalid_ioprio,
+			         value);
+	}
+
 	return value;
 }
 

daemon/daemon_config.h

@@ -44,6 +44,7 @@ POSSIBILITY OF SUCH DAMAGE.
  */
 #define IOPRIO_RESET_DEFAULT -1
 #define IOPRIO_DONT_SET -2
+#define IOPRIO_DEFAULT 4
 
 /*
  * Opaque config context type

daemon/gamemode-ioprio.c

@@ -36,7 +36,9 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "helpers.h"
 #include "logging.h"
 
+#include <dirent.h>
 #include <errno.h>
+#include <stdio.h>
 #include <sys/syscall.h>
 #include <unistd.h>
 
@@ -86,6 +88,25 @@ static inline int ioprio_set(int which, int who, int ioprio)
 	return (int)syscall(SYS_ioprio_set, which, who, ioprio);
 }
 
+static inline int ioprio_get(int which, int who)
+{
+	return (int)syscall(SYS_ioprio_get, which, who);
+}
+
+/**
+ * Get the i/o priorities
+ */
+int game_mode_get_ioprio(const pid_t client)
+{
+	int ret = ioprio_get(IOPRIO_WHO_PROCESS, client);
+	if (ret == -1) {
+		LOG_ERROR("Failed to get ioprio value for [%d] with error %s\n", client, strerror(errno));
+		ret = IOPRIO_DONT_SET;
+	}
+	/* We support only IOPRIO_CLASS_BE as IOPRIO_CLASS_RT required CAP_SYS_ADMIN */
+	return IOPRIO_PRIO_DATA(ret);
+}
+
 /**
  * Apply io priorities
  *
@@ -93,49 +114,80 @@ static inline int ioprio_set(int which, int who, int ioprio)
  * and can possibly reduce lags or latency when a game has to load assets
  * on demand.
  */
-void game_mode_apply_ioprio(const GameModeContext *self, const pid_t client)
+void game_mode_apply_ioprio(const GameModeContext *self, const pid_t client, int expected)
 {
-	GameModeConfig *config = game_mode_config_from_context(self);
+	if (expected == IOPRIO_DONT_SET)
+		/* Silently bail if fed a don't set (invalid) */
+		return;
 
-	LOG_MSG("Setting scheduling policies...\n");
+	GameModeConfig *config = game_mode_config_from_context(self);
 
-	/*
-	 * read configuration "ioprio" (0..7)
-	 */
+	/* read configuration "ioprio" (0..7) */
 	int ioprio = (int)config_get_ioprio_value(config);
-	if (IOPRIO_RESET_DEFAULT == ioprio) {
-		LOG_MSG("IO priority will be reset to default behavior (based on CPU priority).\n");
-		ioprio = 0;
-	} else if (IOPRIO_DONT_SET == ioprio) {
+
+	/* Special value to simply not set the value */
+	if (ioprio == IOPRIO_DONT_SET)
 		return;
-	} else {
-		/* maybe clamp the value */
-		int invalid_ioprio = ioprio;
-		ioprio = CLAMP(0, 7, ioprio);
-		if (ioprio != invalid_ioprio)
-			LOG_ONCE(ERROR,
-			         "IO priority value %d invalid, clamping to %d\n",
-			         invalid_ioprio,
-			         ioprio);
-
-		/* We support only IOPRIO_CLASS_BE as IOPRIO_CLASS_RT required CAP_SYS_ADMIN */
-		ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, ioprio);
+
+	LOG_MSG("Setting ioprio value...\n");
+
+	/* If fed the default, we'll try and reset the value back */
+	if (expected != IOPRIO_DEFAULT) {
+		expected = (int)ioprio;
+		ioprio = IOPRIO_DEFAULT;
 	}
 
-	/*
-	 * Actually apply the io priority
-	 */
-	int c = IOPRIO_PRIO_CLASS(ioprio), p = IOPRIO_PRIO_DATA(ioprio);
-	if (ioprio_set(IOPRIO_WHO_PROCESS, client, ioprio) == 0) {
-		if (0 == ioprio)
-			LOG_MSG("Resetting client [%d] IO priority.\n", client);
-		else
-			LOG_MSG("Setting client [%d] IO priority to (%d,%d).\n", client, c, p);
-	} else {
-		LOG_ERROR("Setting client [%d] IO priority to (%d,%d) failed with error %d, ignoring\n",
-		          client,
-		          c,
-		          p,
-		          errno);
+	/* Open the tasks dir for the client */
+	char tasks[128];
+	snprintf(tasks, sizeof(tasks), "/proc/%d/task", client);
+	DIR *client_task_dir = opendir(tasks);
+	if (client_task_dir == NULL) {
+		LOG_ERROR("Could not inspect tasks for client [%d]! Skipping ioprio optimisation.\n",
+		          client);
+		return;
+	}
+
+	/* Iterate for all tasks of client process */
+	struct dirent *tid_entry;
+	while ((tid_entry = readdir(client_task_dir)) != NULL) {
+		/* Skip . and .. */
+		if (tid_entry->d_name[0] == '.')
+			continue;
+
+		/* task name is the name of the file */
+		int tid = atoi(tid_entry->d_name);
+
+		int current = game_mode_get_ioprio(tid);
+		if (current == IOPRIO_DONT_SET) {
+			/* Couldn't get the ioprio value
+			 * This could simply mean that the thread exited before fetching the ioprio
+			 * So we should continue
+			 */
+		} else if (current != expected) {
+			/* Don't try and adjust the ioprio value if the value we got doesn't match default */
+			LOG_ERROR("Skipping ioprio on client [%d,%d]: ioprio was (%d) but we expected (%d)\n",
+			          client,
+			          tid,
+			          current,
+			          expected);
+		} else {
+			/*
+			 * For now we only support IOPRIO_CLASS_BE
+			 * IOPRIO_CLASS_RT requires CAP_SYS_ADMIN but should be possible with a polkit process
+			 */
+			int p = ioprio;
+			ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, ioprio);
+			if (ioprio_set(IOPRIO_WHO_PROCESS, tid, ioprio) != 0) {
+				/* This could simply mean the thread is gone now, as above */
+				LOG_ERROR(
+				    "Setting client [%d,%d] IO priority to (%d) failed with error %d, ignoring.\n",
+				    client,
+				    tid,
+				    p,
+				    errno);
+			}
+		}
 	}
+
+	closedir(client_task_dir);
 }

daemon/gamemode-sched.c

@@ -35,8 +35,10 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "gamemode.h"
 #include "logging.h"
 
+#include <dirent.h>
 #include <errno.h>
 #include <sched.h>
+#include <stdio.h>
 #include <string.h>
 #include <sys/resource.h>
 #include <sys/sysinfo.h>
@@ -54,13 +56,28 @@ POSSIBILITY OF SUCH DAMAGE.
  * This tries to change the scheduler of the client to soft realtime mode
  * available in some kernels as SCHED_ISO. It also tries to adjust the nice
  * level. If some of each fail, ignore this and log a warning.
- *
- * We don't need to store the current values because when the client exits,
- * everything will be good: Scheduling is only applied to the client and
- * its children.
  */
-void game_mode_apply_renice(const GameModeContext *self, const pid_t client)
+
+#define RENICE_INVALID -128 /* Special value to store invalid value */
+int game_mode_get_renice(const pid_t client)
 {
+	/* Clear errno as -1 is a regitimate return */
+	errno = 0;
+	int priority = getpriority(PRIO_PROCESS, (id_t)client);
+	if (priority == -1 && errno) {
+		LOG_ERROR("getprority(PRIO_PROCESS, %d) failed : %s\n", client, strerror(errno));
+		return RENICE_INVALID;
+	}
+	return -priority;
+}
+
+/* If expected is 0 then we try to apply our renice, otherwise, we try to remove it */
+void game_mode_apply_renice(const GameModeContext *self, const pid_t client, int expected)
+{
+	if (expected == RENICE_INVALID)
+		/* Silently bail if fed an invalid value */
+		return;
+
 	GameModeConfig *config = game_mode_config_from_context(self);
 
 	/*
@@ -69,26 +86,70 @@ void game_mode_apply_renice(const GameModeContext *self, const pid_t client)
 	long int renice = config_get_renice_value(config);
 	if (renice == 0) {
 		return;
-	} else if ((renice < 1) || (renice > 20)) {
-		LOG_ONCE(ERROR, "Configured renice value '%ld' is invalid, will not renice.\n", renice);
+	}
+
+	/* Invert the renice value */
+	renice = -renice;
+
+	/* When expected is non-zero, we should try and remove the renice only if it doesn't match the
+	 * expected value */
+	if (expected != 0) {
+		expected = (int)renice;
+		renice = 0;
+	}
+
+	/* Open the tasks dir for the client */
+	char tasks[128];
+	snprintf(tasks, sizeof(tasks), "/proc/%d/task", client);
+	DIR *client_task_dir = opendir(tasks);
+	if (client_task_dir == NULL) {
+		LOG_ERROR("Could not inspect tasks for client [%d]! Skipping ioprio optimisation.\n",
+		          client);
 		return;
-	} else {
-		renice = -renice;
 	}
 
-	/*
-	 * don't adjust priority if it was already adjusted
-	 */
-	if (getpriority(PRIO_PROCESS, (id_t)client) != 0) {
-		LOG_ERROR("Refused to renice client [%d]: already reniced\n", client);
-	} else if (setpriority(PRIO_PROCESS, (id_t)client, (int)renice)) {
-		LOG_HINTED(ERROR,
-		           "Failed to renice client [%d], ignoring error condition: %s\n",
-		           "    -- Your user may not have permission to do this. Please read the docs\n"
-		           "    -- to learn how to adjust the pam limits.\n",
-		           client,
-		           strerror(errno));
+	/* Iterate for all tasks of client process */
+	struct dirent *tid_entry;
+	while ((tid_entry = readdir(client_task_dir)) != NULL) {
+		/* Skip . and .. */
+		if (tid_entry->d_name[0] == '.')
+			continue;
+
+		/* task name is the name of the file */
+		int tid = atoi(tid_entry->d_name);
+
+		/* Clear errno as -1 is a regitimate return */
+		errno = 0;
+		int prio = getpriority(PRIO_PROCESS, (id_t)tid);
+
+		if (prio == -1 && errno) {
+			/* Process may well have ended */
+			LOG_ERROR("getpriority failed for client [%d,%d] with error: %s\n",
+			          client,
+			          tid,
+			          strerror(errno));
+		} else if (prio != expected) {
+			/*
+			 * Don't adjust priority if it does not match the expected value
+			 * ie. Another process has changed it, or it began non-standard
+			 */
+			LOG_ERROR("Refused to renice client [%d,%d]: prio was (%d) but we expected (%d)\n",
+			          client,
+			          tid,
+			          prio,
+			          expected);
+		} else if (setpriority(PRIO_PROCESS, (id_t)tid, (int)renice)) {
+			LOG_HINTED(ERROR,
+			           "Failed to renice client [%d,%d], ignoring error condition: %s\n",
+			           "    -- Your user may not have permission to do this. Please read the docs\n"
+			           "    -- to learn how to adjust the pam limits.\n",
+			           client,
+			           tid,
+			           strerror(errno));
+		}
 	}
+
+	closedir(client_task_dir);
 }
 
 void game_mode_apply_scheduling(const GameModeContext *self, const pid_t client)

daemon/gamemode-tests.c

@@ -36,6 +36,8 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "logging.h"
 
 #include <libgen.h>
+#include <pthread.h>
+#include <sys/syscall.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
@@ -536,6 +538,249 @@ int run_gpu_optimisation_tests(struct GameModeConfig *config)
 	return gpustatus;
 }
 
+/**
+ * Multithreaded process simulation
+ *
+ * Some of the optimisations that gamemode implements needs to be tested against a full process
+ * tree, otherwise we may only be applying them to only the main thread
+ */
+typedef struct {
+	pthread_barrier_t *barrier;
+	pid_t this;
+} ThreadInfo;
+
+static void *fake_thread_wait(void *arg)
+{
+	ThreadInfo *info = (ThreadInfo *)arg;
+
+	/* Store the thread ID */
+	info->this = (pid_t)syscall(SYS_gettid);
+
+	/**
+	 * Wait twice
+	 * First to sync that all threads have started
+	 * Second to sync all threads exiting
+	 */
+	int ret = 0;
+	ret = pthread_barrier_wait(info->barrier);
+	if (ret != 0 && ret != PTHREAD_BARRIER_SERIAL_THREAD)
+		FATAL_ERROR("pthread_barrier_wait failed in child with error %d!\n", ret);
+
+	ret = pthread_barrier_wait(info->barrier);
+	if (ret != 0 && ret != PTHREAD_BARRIER_SERIAL_THREAD)
+		FATAL_ERROR("pthread_barrier_wait failed in child with error %d!\n", ret);
+
+	return NULL;
+}
+
+/* Runs a process tree in a child and tests each thread */
+static pid_t run_tests_on_process_tree(int innactive, int active, int (*func)(pid_t))
+{
+	/* Create a fake game-like multithreaded fork */
+	pid_t child = fork();
+	if (child == 0) {
+		/* Some stetup */
+		bool fail = false;
+		const unsigned int numthreads = 3;
+		pthread_barrier_t barrier;
+		pthread_barrier_init(&barrier, NULL, numthreads + 1);
+
+		/* First, request gamemode for this child process before it created the threads */
+		gamemode_request_start();
+
+		/* Spawn a few child threads */
+		pthread_t threads[numthreads];
+		ThreadInfo info[numthreads];
+		for (unsigned int i = 0; i < numthreads; i++) {
+			info[i].barrier = &barrier;
+			int err = pthread_create(&threads[i], NULL, fake_thread_wait, &info[i]);
+			if (err != 0) {
+				LOG_ERROR("Failed to spawn thread! Error: %d\n", err);
+				exit(EXIT_FAILURE);
+			}
+		}
+
+		/* Wait for threads to be created */
+		pthread_barrier_wait(&barrier);
+
+		/* Test each spawned thread */
+		for (unsigned int i = 0; i < numthreads; i++)
+			fail |= (active != func(info[i].this));
+
+		if (fail) {
+			LOG_ERROR("Initial values for new threads were incorrect!\n");
+			gamemode_request_end();
+			exit(-1);
+		}
+
+		/* Request gamemode end */
+		gamemode_request_end();
+
+		/* Test each spawned thread */
+		for (unsigned int i = 0; i < numthreads; i++)
+			fail |= (innactive != func(info[i].this));
+		if (fail) {
+			LOG_ERROR("values for threads were not reset after gamemode_request_end!\n");
+			exit(-1);
+		}
+
+		/* Request gamemode again - this time after threads were created */
+		gamemode_request_start();
+
+		/* Test each spawned thread */
+		for (unsigned int i = 0; i < numthreads; i++)
+			fail |= (active != func(info[i].this));
+		if (fail) {
+			LOG_ERROR("values for threads were not set correctly!\n");
+			gamemode_request_end();
+			exit(-1);
+		}
+
+		/* Request gamemode end */
+		gamemode_request_end();
+
+		/* Test each spawned thread */
+		for (unsigned int i = 0; i < numthreads; i++)
+			fail |= (innactive != func(info[i].this));
+		if (fail) {
+			LOG_ERROR("values for threads were not reset after gamemode_request_end!\n");
+			exit(-1);
+		}
+
+		/* Tell the threads to continue */
+		pthread_barrier_wait(&barrier);
+
+		/* Wait for threads to join */
+		int ret = 0;
+		for (unsigned int i = 0; i < numthreads; i++)
+			ret &= pthread_join(threads[i], NULL);
+
+		if (ret != 0)
+			LOG_ERROR("Thread cleanup in multithreaded tests failed!\n");
+
+		/* We're done, so return the error code generated */
+		exit(ret);
+	}
+
+	/* Wait for the child */
+	int wstatus = 0;
+	waitpid(child, &wstatus, 0);
+
+	int status = 0;
+	if (WIFEXITED(wstatus))
+		status = WEXITSTATUS(wstatus);
+	else {
+		LOG_ERROR("Multithreaded child exited abnormally!\n");
+		status = -1;
+	}
+
+	return status;
+}
+
+int run_renice_tests(struct GameModeConfig *config)
+{
+	/* read configuration "renice" (1..20) */
+	long int renice = config_get_renice_value(config);
+	if (renice == 0) {
+		return 1; /* not configured */
+	}
+
+	/* Verify renice starts at 0 */
+	int val = game_mode_get_renice(getpid());
+	if (val != 0) {
+		LOG_ERROR("Initial renice value is non-zero: %d\n", val);
+		return -1;
+	}
+
+	int ret = 0;
+
+	/* Ask for gamemode for ourselves */
+	gamemode_request_start();
+
+	/* Check renice is now requested value */
+	val = game_mode_get_renice(getpid());
+	if (val != renice) {
+		LOG_ERROR(
+		    "renice value not set correctly after gamemode_request_start\nExpected: %ld, Was: %d\n",
+		    renice,
+		    val);
+		ret = -1;
+	}
+
+	/* End gamemode for ourselves */
+	gamemode_request_end();
+
+	/* Check renice is returned to correct value */
+	val = game_mode_get_renice(getpid());
+	if (val != 0) {
+		LOG_ERROR("renice value non-zero after gamemode_request_end\nExpected: 0, Was: %d\n", val);
+		ret = -1;
+	}
+
+	/* Check multiprocess nice works as well */
+	val = run_tests_on_process_tree(0, (int)renice, game_mode_get_renice);
+	if (val != 0) {
+		LOG_ERROR("Multithreaded renice tests failed!\n");
+		ret = -1;
+	}
+
+	return ret;
+}
+
+int run_ioprio_tests(struct GameModeConfig *config)
+{
+	/* read configuration "ioprio" */
+	long int ioprio = config_get_ioprio_value(config);
+	if (ioprio == IOPRIO_DONT_SET) {
+		return 1; /* not configured */
+	}
+
+	/* Verify ioprio starts at 0 */
+	int val = game_mode_get_ioprio(getpid());
+	if (val != IOPRIO_DEFAULT) {
+		LOG_ERROR("Initial ioprio value is non-default\nExpected: %d, Was: %d\n",
+		          IOPRIO_DEFAULT,
+		          val);
+		return -1;
+	}
+
+	int ret = 0;
+
+	/* Ask for gamemode for ourselves */
+	gamemode_request_start();
+
+	/* Check renice is now requested value */
+	val = game_mode_get_ioprio(getpid());
+	if (val != ioprio) {
+		LOG_ERROR(
+		    "ioprio value not set correctly after gamemode_request_start\nExpected: %ld, Was: %d\n",
+		    ioprio,
+		    val);
+		ret = -1;
+	}
+
+	/* End gamemode for ourselves */
+	gamemode_request_end();
+
+	/* Check ioprio is returned to correct value */
+	val = game_mode_get_ioprio(getpid());
+	if (val != IOPRIO_DEFAULT) {
+		LOG_ERROR("ioprio value non-default after gamemode_request_end\nExpected: %d, Was: %d\n",
+		          IOPRIO_DEFAULT,
+		          val);
+		ret = -1;
+	}
+
+	/* Check multiprocess nice works as well */
+	val = run_tests_on_process_tree(IOPRIO_DEFAULT, (int)ioprio, game_mode_get_ioprio);
+	if (val != 0) {
+		LOG_ERROR("Multithreaded ioprio tests failed!\n");
+		ret = -1;
+	}
+
+	return ret;
+}
+
 /**
  * game_mode_run_feature_tests runs a set of tests for each current feature (based on the current
  * config) returns 0 for success, -1 for failure
@@ -558,7 +803,7 @@ static int game_mode_run_feature_tests(struct GameModeConfig *config)
 		else {
 			LOG_MSG("::: Failed!\n");
 			// Consider the CPU governor feature required
-			status = 1;
+			status = -1;
 		}
 	}
 
@@ -574,7 +819,7 @@ static int game_mode_run_feature_tests(struct GameModeConfig *config)
 		else {
 			LOG_MSG("::: Failed!\n");
 			// Any custom scripts should be expected to work
-			status = 1;
+			status = -1;
 		}
 	}
 
@@ -590,18 +835,45 @@ static int game_mode_run_feature_tests(struct GameModeConfig *config)
 		else {
 			LOG_MSG("::: Failed!\n");
 			// Any custom scripts should be expected to work
-			status = 1;
+			status = -1;
 		}
 	}
 
-	/* Does the screensaver get inhibited? */
-	/* TODO: Unknown if this is testable, org.freedesktop.ScreenSaver has no query method */
-
 	/* Was the process reniced? */
-	/* Was the scheduling applied? */
-	/* Were io priorities changed? */
-	/* Note: These don't get cleared up on un-register, so will have already been applied */
+	{
+		LOG_MSG("::: Verifying renice\n");
+		int renicestatus = run_renice_tests(config);
+
+		if (renicestatus == 1)
+			LOG_MSG("::: Passed (no renice configured)\n");
+		else if (renicestatus == 0)
+			LOG_MSG("::: Passed\n");
+		else {
+			LOG_MSG("::: Failed!\n");
+			// Renice should be expected to work, if set
+			status = -1;
+		}
+	}
+
+	/* Was the process ioprio set? */
+	{
+		LOG_MSG("::: Verifying ioprio\n");
+		int iopriostatus = run_ioprio_tests(config);
+
+		if (iopriostatus == 1)
+			LOG_MSG("::: Passed (no ioprio configured)\n");
+		else if (iopriostatus == 0)
+			LOG_MSG("::: Passed\n");
+		else {
+			LOG_MSG("::: Failed!\n");
+			status = -1;
+		}
+	}
+
 	/* TODO */
+	/* Was the scheduling applied and removed? Does it get applied to a full process tree? */
+	/* Does the screensaver get inhibited? Unknown if this is testable, org.freedesktop.ScreenSaver
+	 * has no query method */
 
 	if (status != -1)
 		LOG_MSG(":: Passed%s\n\n", status > 0 ? " (with optional failures)" : "");
@@ -727,7 +999,6 @@ int game_mode_run_client_tests(void)
 		return -1;
 
 	/* Controls whether we require a supervisor to actually make requests */
-	/* TODO: This effects all tests below */
 	if (config_get_require_supervisor(config) != 0) {
 		LOG_ERROR("Tests currently unsupported when require_supervisor is set\n");
 		return -1;

daemon/gamemode.c

@@ -412,13 +412,15 @@ int game_mode_context_register(GameModeContext *self, pid_t client, pid_t reques
 		game_mode_context_enter(self);
 	}
 
+	/* Store current renice and apply */
+	game_mode_apply_renice(self, client, 0 /* expect zero value to start with */);
+
+	/* Store current ioprio value and apply  */
+	game_mode_apply_ioprio(self, client, IOPRIO_DEFAULT);
+
 	/* Apply scheduler policies */
-	game_mode_apply_renice(self, client);
 	game_mode_apply_scheduling(self, client);
 
-	/* Apply io priorities */
-	game_mode_apply_ioprio(self, client);
-
 	game_mode_client_count_changed();
 
 	return 0;
@@ -506,6 +508,12 @@ int game_mode_context_unregister(GameModeContext *self, pid_t client, pid_t requ
 
 	game_mode_client_count_changed();
 
+	/* Restore the ioprio value for the process, expecting it to be the config value  */
+	game_mode_apply_ioprio(self, client, (int)config_get_ioprio_value(self->config));
+
+	/* Restore the renice value for the process, expecting it to be our config value */
+	game_mode_apply_renice(self, client, (int)config_get_renice_value(self->config));
+
 	return 0;
 }
 

daemon/gamemode.h

@@ -122,7 +122,8 @@ char *game_mode_lookup_user_home(void);
  * Provides internal API functions specific to adjusting process
  * IO priorities.
  */
-void game_mode_apply_ioprio(const GameModeContext *self, const pid_t client);
+int game_mode_get_ioprio(const pid_t client);
+void game_mode_apply_ioprio(const GameModeContext *self, const pid_t client, int expected);
 
 /** gamemode-proc.c
  * Provides internal API functions specific to working with process
@@ -135,7 +136,8 @@ int game_mode_close_proc(const procfd_t procfd);
  * Provides internal API functions specific to adjusting process
  * scheduling.
  */
-void game_mode_apply_renice(const GameModeContext *self, const pid_t client);
+int game_mode_get_renice(const pid_t client);
+void game_mode_apply_renice(const GameModeContext *self, const pid_t client, int expected);
 void game_mode_apply_scheduling(const GameModeContext *self, const pid_t client);
 
 /** gamemode-wine.c

daemon/helpers.h

@@ -39,7 +39,7 @@ POSSIBILITY OF SUCH DAMAGE.
 /**
  * Value clamping helper, works like MIN/MAX but constraints a value within the range.
  */
-#define CLAMP(lbound, ubound, value) MIN(MIN(lbound, ubound), MAX(MAX(lbound, ubound), value))
+#define CLAMP(l, u, value) MAX(MIN(l, u), MIN(MAX(l, u), value))
 
 /**
  * Little helper to safely print into a buffer, returns a pointer into the buffer

daemon/main.c

@@ -56,6 +56,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "gamemode_client.h"
 #include "logging.h"
 
+#include <getopt.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <string.h>
@@ -64,12 +65,15 @@ POSSIBILITY OF SUCH DAMAGE.
 
 #define USAGE_TEXT                                                                                 \
 	"Usage: %s [-d] [-l] [-r] [-t] [-h] [-v]\n\n"                                                  \
-	"  -d  daemonize self after launch\n"                                                          \
-	"  -l  log to syslog\n"                                                                        \
-	"  -r  request gamemode and pause\n"                                                           \
-	"  -t  run tests\n"                                                                            \
-	"  -h  print this help\n"                                                                      \
-	"  -v  print version\n"                                                                        \
+	"  -r[PID], --request=[PID] Toggle gamemode for process\n"                                     \
+	"                           When no PID given, requests gamemode and pauses\n"                 \
+	"  -s[PID], --status=[PID]  Query the status of gamemode for process\n"                        \
+	"                           When no PID given, queries the status globally\n"                  \
+	"  -d, --daemonize          Daemonize self after launch\n"                                     \
+	"  -l, --log-to-syslog      Log to syslog\n"                                                   \
+	"  -r, --test               Run tests\n"                                                       \
+	"  -h, --help               Print this help\n"                                                 \
+	"  -v, --version            Print version\n"                                                   \
 	"\n"                                                                                           \
 	"See man page for more information.\n"
 
@@ -102,8 +106,17 @@ int main(int argc, char *argv[])
 	bool daemon = false;
 	bool use_syslog = false;
 	int opt = 0;
-	int status;
-	while ((opt = getopt(argc, argv, "dlsrtvh")) != -1) {
+
+	/* Options struct for getopt_long */
+	static struct option long_options[] = {
+		{ "daemonize", no_argument, 0, 'd' },     { "log-to-syslog", no_argument, 0, 'l' },
+		{ "request", optional_argument, 0, 'r' }, { "test", no_argument, 0, 't' },
+		{ "status", optional_argument, 0, 's' },  { "help", no_argument, 0, 'h' },
+		{ "version", no_argument, 0, 'v' },       { NULL, 0, NULL, 0 },
+	};
+	static const char *short_options = "dls::r::tvh";
+
+	while ((opt = getopt_long(argc, argv, short_options, long_options, 0)) != -1) {
 		switch (opt) {
 		case 'd':
 			daemon = true;
@@ -111,65 +124,135 @@ int main(int argc, char *argv[])
 		case 'l':
 			use_syslog = true;
 			break;
-		case 's': {
-			if ((status = gamemode_query_status()) < 0) {
-				LOG_ERROR("gamemode status request failed: %s\n", gamemode_error_string());
-				exit(EXIT_FAILURE);
-			} else if (status > 0) {
-				LOG_MSG("gamemode is active\n");
+
+		case 's':
+			if (optarg != NULL) {
+				pid_t pid = atoi(optarg);
+				switch (gamemode_query_status_for(pid)) {
+				case 0: /* inactive */
+					LOG_MSG("gamemode is inactive\n");
+					break;
+				case 1: /* active not not registered */
+					LOG_MSG("gamemode is active but [%d] not registered\n", pid);
+					break;
+				case 2: /* active for client */
+					LOG_MSG("gamemode is active and [%d] registered\n", pid);
+					break;
+				case -1:
+					LOG_ERROR("gamemode_query_status_for(%d) failed: %s\n",
+					          pid,
+					          gamemode_error_string());
+					exit(EXIT_FAILURE);
+				default:
+					LOG_ERROR("gamemode_query_status returned unexpected value 2\n");
+					exit(EXIT_FAILURE);
+				}
 			} else {
-				LOG_MSG("gamemode is inactive\n");
+				int ret = 0;
+				switch ((ret = gamemode_query_status())) {
+				case 0: /* inactive */
+					LOG_MSG("gamemode is inactive\n");
+					break;
+				case 1: /* active */
+					LOG_MSG("gamemode is active\n");
+					break;
+				case -1: /* error */
+					LOG_ERROR("gamemode status request failed: %s\n", gamemode_error_string());
+					exit(EXIT_FAILURE);
+				default: /* unexpected value eg. 2 */
+					LOG_ERROR("gamemode_query_status returned unexpected value %d\n", ret);
+					exit(EXIT_FAILURE);
+				}
 			}
 
 			exit(EXIT_SUCCESS);
-			break;
-		}
+
 		case 'r':
-			if (gamemode_request_start() < 0) {
-				LOG_ERROR("gamemode request failed: %s\n", gamemode_error_string());
-				exit(EXIT_FAILURE);
-			}
 
-			if ((status = gamemode_query_status()) == 2) {
-				LOG_MSG("gamemode request succeeded and is active\n");
-			} else if (status == 1) {
-				LOG_ERROR("gamemode request succeeded and is active but registration failed\n");
-				exit(EXIT_FAILURE);
+			if (optarg != NULL) {
+				pid_t pid = atoi(optarg);
+
+				/* toggle gamemode for the process */
+				switch (gamemode_query_status_for(pid)) {
+				case 0: /* inactive */
+				case 1: /* active not not registered */
+					LOG_MSG("gamemode not active for client, requesting start for %d...\n", pid);
+					if (gamemode_request_start_for(pid) < 0) {
+						LOG_ERROR("gamemode_request_start_for(%d) failed: %s\n",
+						          pid,
+						          gamemode_error_string());
+						exit(EXIT_FAILURE);
+					}
+					LOG_MSG("request succeeded\n");
+					break;
+				case 2: /* active for client */
+					LOG_MSG("gamemode active for client, requesting end for %d...\n", pid);
+					if (gamemode_request_end_for(pid) < 0) {
+						LOG_ERROR("gamemode_request_end_for(%d) failed: %s\n",
+						          pid,
+						          gamemode_error_string());
+						exit(EXIT_FAILURE);
+					}
+					LOG_MSG("request succeeded\n");
+					break;
+				case -1: /* error */
+					LOG_ERROR("gamemode_query_status_for(%d) failed: %s\n",
+					          pid,
+					          gamemode_error_string());
+					exit(EXIT_FAILURE);
+				}
+
 			} else {
-				LOG_ERROR("gamemode request succeeded but is not active\n");
-				exit(EXIT_FAILURE);
-			}
+				/* Request gamemode for this process */
+				if (gamemode_request_start() < 0) {
+					LOG_ERROR("gamemode request failed: %s\n", gamemode_error_string());
+					exit(EXIT_FAILURE);
+				}
 
-			// Simply pause and wait a SIGINT
-			if (signal(SIGINT, sigint_handler_noexit) == SIG_ERR) {
-				FATAL_ERRORNO("Could not catch SIGINT");
-			}
-			pause();
+				/* Request and report on the status */
+				switch (gamemode_query_status()) {
+				case 2: /* active for this client */
+					LOG_MSG("gamemode request succeeded and is active\n");
+					break;
+				case 1: /* active */
+					LOG_ERROR("gamemode request succeeded and is active but registration failed\n");
+					exit(EXIT_FAILURE);
+				case 0: /* innactive */
+					LOG_ERROR("gamemode request succeeded but is not active\n");
+					exit(EXIT_FAILURE);
+				case -1: /* error */
+					LOG_ERROR("gamemode_query_status failed: %s\n", gamemode_error_string());
+					exit(EXIT_FAILURE);
+				}
+
+				/* Simply pause and wait a SIGINT */
+				if (signal(SIGINT, sigint_handler_noexit) == SIG_ERR) {
+					FATAL_ERRORNO("Could not catch SIGINT");
+				}
+				pause();
 
-			// Explicitly clean up
-			if (gamemode_request_end() < 0) {
-				LOG_ERROR("gamemode request failed: %s\n", gamemode_error_string());
-				exit(EXIT_FAILURE);
+				/* Explicitly clean up */
+				if (gamemode_request_end() < 0) {
+					LOG_ERROR("gamemode request failed: %s\n", gamemode_error_string());
+					exit(EXIT_FAILURE);
+				}
 			}
 
 			exit(EXIT_SUCCESS);
-			break;
-		case 't':
-			status = game_mode_run_client_tests();
+
+		case 't': {
+			int status = game_mode_run_client_tests();
 			exit(status);
-			break;
+		}
 		case 'v':
 			LOG_MSG(VERSION_TEXT);
 			exit(EXIT_SUCCESS);
-			break;
 		case 'h':
 			LOG_MSG(USAGE_TEXT, argv[0]);
 			exit(EXIT_SUCCESS);
-			break;
 		default:
 			fprintf(stderr, USAGE_TEXT, argv[0]);
 			exit(EXIT_FAILURE);
-			break;
 		}
 	}
 

data/gamemoded.8.in

@@ -4,7 +4,7 @@
 .SH NAME
 gamemoded \- optimises system performance on demand
 .SH SYNOPSIS
-\fBgamemoded\fR [\fB\-d\fR] [\fB\-l\fR] [\fB\-h\fR] [\fB\-v\fR]
+\fBgamemoded\fR [OPTIONS...]
 .SH DESCRIPTION
 \fBGameMode\fR is a daemon/lib combo for Linux that allows games to request a set of optimisations be temporarily applied to the host OS.
 
@@ -12,26 +12,29 @@ The design has a clear cut abstraction between the host daemon and library (\fBg
 
 \fBGameMode\fR was designed primarily as a stop-gap solution to problems with the Intel and AMD CPU powersave or ondemand governors, but is intended to be expanded beyond just CPU governor states, as there are a wealth of automation tasks one might want to apply.
 .SH OPTIONS
+
 .TP 8
-.B \-d
-Run the daemon as a separate process (daemonize it)
-.TP 8
-.B \-l
-Log to syslog
+.B \-r[PID], \-\-request=[PID]
+Toggle gamemode for process.
+When no PID given, requests gamemode and pauses
 .TP 8
-.B \-r
-Request gamemode and wait for any signal
+.B \-s[PID], \-\-status=[PID]
+Query the status of gamemode for process
+When no PID given, queries the status globally
 .TP 8
-.B \-s
-Query the current status of gamemode
+.B \-d, \-\-daemonize
+Run the daemon as a separate process (daemonize it)
 .TP 8
-.B \-h
+.B \-l, \-\-log-to-syslog
+Log to syslog
+.TP 8 
+.B \-h, \-\-help
 Print help text
 .TP 8
-.B \-t
+.B \-t, \-\-test
 Run diagnostic tests on the current installation
 .TP 8
-.B \-v
+.B \-v, \-\-version
 Print the version
 
 .SH USAGE

lib/gamemode_client.h

@@ -58,9 +58,11 @@ POSSIBILITY OF SUCH DAMAGE.
  *   -1 if the request failed
  *   -2 if the request was rejected
  *
- * int gamemode_query_status_for(pid_t pid) - Query the current status of gamemode for another
- * process 0 if gamemode is inactive 1 if gamemode is active 2 if gamemode is active and this client
- * is registered -1 if the query failed
+ * int gamemode_query_status_for(pid_t pid) - Query status of gamemode for another process
+ *   0 if gamemode is inactive
+ *   1 if gamemode is active
+ *   2 if gamemode is active and this client is registered
+ *   -1 if the query failed
  *
  * const char* gamemode_error_string() - Get an error string
  *   returns a string describing any of the above errors