Set up for resetting niceness value

Add tests to check this feature
	Apply config validation in config for simplicity

	Note: if anything messes with the niceness (ie. it starts non-zero, or it's not the expected value during setup, we'll bail out)

daemon/daemon_config.c

@@ -548,6 +548,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;
 }
 

daemon/gamemode-sched.c

@@ -54,13 +54,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,18 +84,35 @@ 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);
-		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);
+	/* 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;
+	}
+
+	/* Clear errno as -1 is a regitimate return */
+	errno = 0;
+	int prio = getpriority(PRIO_PROCESS, (id_t)client);
+	if (prio == -1 && errno) {
+		/* process has likely ended, only log an error if we were actually trying to set a non-zero
+		 * value */
+		if (errno == ESRCH && renice != 0)
+			LOG_ERROR("getpriority returned ESRCH for process %d\n", client);
+	} 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]: prio was (%d) but we expected (%d)\n",
+		          client,
+		          prio,
+		          expected);
 	} else if (setpriority(PRIO_PROCESS, (id_t)client, (int)renice)) {
 		LOG_HINTED(ERROR,
 		           "Failed to renice client [%d], ignoring error condition: %s\n",

daemon/gamemode-tests.c

@@ -536,6 +536,49 @@ int run_gpu_optimisation_tests(struct GameModeConfig *config)
 	return gpustatus;
 }
 
+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;
+	}
+
+	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
@@ -598,6 +641,21 @@ static int game_mode_run_feature_tests(struct GameModeConfig *config)
 	/* TODO: Unknown if this is testable, org.freedesktop.ScreenSaver has no query method */
 
 	/* Was the process reniced? */
+	{
+		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 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

@@ -71,6 +71,8 @@ struct GameModeContext {
 	struct GameModeGPUInfo *stored_gpu; /**<Stored GPU info for the current GPU */
 	struct GameModeGPUInfo *target_gpu; /**<Target GPU info for the current GPU */
 
+	int initial_renice; /**<Initial renice value */
+
 	/* Reaper control */
 	struct {
 		pthread_t thread;
@@ -412,8 +414,11 @@ int game_mode_context_register(GameModeContext *self, pid_t client, pid_t reques
 		game_mode_context_enter(self);
 	}
 
+	/* Store current renice and apply */
+	self->initial_renice = game_mode_get_renice(client);
+	game_mode_apply_renice(self, client, 0 /* expect zero value to start with */);
+
 	/* Apply scheduler policies */
-	game_mode_apply_renice(self, client);
 	game_mode_apply_scheduling(self, client);
 
 	/* Apply io priorities */

daemon/gamemode.h

@@ -135,7 +135,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