diff --git a/daemon/daemon_config.c b/daemon/daemon_config.c index 23e0e76..cb2a7eb 100644 --- a/daemon/daemon_config.c +++ b/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; } diff --git a/daemon/gamemode-sched.c b/daemon/gamemode-sched.c index 698ecd1..e4f2019 100644 --- a/daemon/gamemode-sched.c +++ b/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", diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c index 71f2a9d..f34a613 100644 --- a/daemon/gamemode-tests.c +++ b/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 */ diff --git a/daemon/gamemode.c b/daemon/gamemode.c index e8fab8b..ff19799 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -71,6 +71,8 @@ struct GameModeContext { struct GameModeGPUInfo *stored_gpu; /**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 */ diff --git a/daemon/gamemode.h b/daemon/gamemode.h index 7a7703c..61003fa 100644 --- a/daemon/gamemode.h +++ b/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