Get ready for re-setting ioprio value on un-register

	Implements tests for feature

	Fixes CLAMP macro

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 */
@@ -564,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

@@ -86,6 +86,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 +112,51 @@ 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",
+	int current = game_mode_get_ioprio(client);
+	if (current == IOPRIO_DONT_SET) {
+		/* Couldn't get the ioprio value, let's bail */
+		return;
+	} else if (current != expected) {
+		/* Don't try and adjust the ioprio value if the value we got doesn't match default */
+		LOG_ERROR("Refused to set ioprio on client [%d]: prio was (%d) but we expected (%d)\n",
 		          client,
-		          c,
-		          p,
-		          errno);
+		          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, client, ioprio) != 0) {
+			LOG_ERROR("Setting client [%d] IO priority to (%d) failed with error %d, ignoring\n",
+			          client,
+			          p,
+			          errno);
+		}
 	}
 }

daemon/gamemode-tests.c

@@ -579,6 +579,53 @@ int run_renice_tests(struct GameModeConfig *config)
 	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;
+	}
+
+	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
@@ -656,6 +703,22 @@ static int game_mode_run_feature_tests(struct GameModeConfig *config)
 		}
 	}
 
+	/* 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");
+			// Ioprio should be expected to work, if set
+			status = 1;
+		}
+	}
+
 	/* Was the scheduling applied? */
 	/* Were io priorities changed? */
 	/* Note: These don't get cleared up on un-register, so will have already been applied */

daemon/gamemode.c

@@ -72,6 +72,7 @@ struct GameModeContext {
 	struct GameModeGPUInfo *target_gpu; /**<Target GPU info for the current GPU */
 
 	int initial_renice; /**<Initial renice value */
+	int initial_ioprio; /**<Initial ioprio value */
 
 	/* Reaper control */
 	struct {
@@ -418,12 +419,13 @@ int game_mode_context_register(GameModeContext *self, pid_t client, pid_t reques
 	self->initial_renice = game_mode_get_renice(client);
 	game_mode_apply_renice(self, client, 0 /* expect zero value to start with */);
 
+	/* Store current ioprio value and apply  */
+	self->initial_ioprio = game_mode_get_ioprio(client);
+	game_mode_apply_ioprio(self, client, IOPRIO_DEFAULT);
+
 	/* Apply scheduler policies */
 	game_mode_apply_scheduling(self, client);
 
-	/* Apply io priorities */
-	game_mode_apply_ioprio(self, client);
-
 	game_mode_client_count_changed();
 
 	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

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