diff --git a/daemon/daemon_config.c b/daemon/daemon_config.c index cb2a7eb..a03147b 100644 --- a/daemon/daemon_config.c +++ b/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; } diff --git a/daemon/daemon_config.h b/daemon/daemon_config.h index 7418d07..790d26c 100644 --- a/daemon/daemon_config.h +++ b/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 diff --git a/daemon/gamemode-ioprio.c b/daemon/gamemode-ioprio.c index f287421..544be9f 100644 --- a/daemon/gamemode-ioprio.c +++ b/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) { + if (expected == IOPRIO_DONT_SET) + /* Silently bail if fed a don't set (invalid) */ + return; + GameModeConfig *config = game_mode_config_from_context(self); - LOG_MSG("Setting scheduling policies...\n"); - - /* - * 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) { - 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); + /* Special value to simply not set the value */ + if (ioprio == IOPRIO_DONT_SET) + return; + + 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); + } } } diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c index f34a613..2d4c54a 100644 --- a/daemon/gamemode-tests.c +++ b/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 */ diff --git a/daemon/gamemode.c b/daemon/gamemode.c index 8823c89..d9c1708 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -72,6 +72,7 @@ struct GameModeContext { struct GameModeGPUInfo *target_gpu; /**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; diff --git a/daemon/gamemode.h b/daemon/gamemode.h index 61003fa..d5d6eb6 100644 --- a/daemon/gamemode.h +++ b/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 diff --git a/daemon/helpers.h b/daemon/helpers.h index 8595092..f8dfc1a 100644 --- a/daemon/helpers.h +++ b/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