diff --git a/daemon/daemon_config.c b/daemon/daemon_config.c index 1183433..96db7a7 100644 --- a/daemon/daemon_config.c +++ b/daemon/daemon_config.c @@ -71,6 +71,7 @@ struct GameModeConfig { char whitelist[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]; char blacklist[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]; + long script_timeout; char startscripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]; char endscripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]; @@ -278,6 +279,8 @@ static int inih_handler(void *user, const char *section, const char *name, const valid = append_value_to_list(name, value, self->values.startscripts); } else if (strcmp(name, "end") == 0) { valid = append_value_to_list(name, value, self->values.endscripts); + } else if (strcmp(name, "script_timeout") == 0) { + valid = get_long_value(name, value, &self->values.script_timeout); } } @@ -330,6 +333,7 @@ static void load_config_files(GameModeConfig *self) self->values.reaper_frequency = DEFAULT_REAPER_FREQ; self->values.gpu_device = -1; /* 0 is a valid device ID so use -1 to indicate no value */ self->values.nv_perf_level = -1; + self->values.script_timeout = 10; /* Default to 10 seconds for scripts */ /* * Locations to load, in order @@ -506,6 +510,11 @@ void config_get_gamemode_end_scripts(GameModeConfig *self, memcpy_locked_config(self, scripts, self->values.endscripts, sizeof(self->values.startscripts)); } +/* + * Get the script timemout value + */ +DEFINE_CONFIG_GET(script_timeout) + /* * Get the chosen default governor */ diff --git a/daemon/daemon_config.h b/daemon/daemon_config.h index 2981e8d..d52da9b 100644 --- a/daemon/daemon_config.h +++ b/daemon/daemon_config.h @@ -105,6 +105,11 @@ void config_get_gamemode_start_scripts(GameModeConfig *self, void config_get_gamemode_end_scripts(GameModeConfig *self, char scripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]); +/* + * Get the script timout value + */ +long config_get_script_timeout(GameModeConfig *self); + /* * Get the chosen default governor */ diff --git a/daemon/external-helper.c b/daemon/external-helper.c index ca19e96..6cb01fd 100644 --- a/daemon/external-helper.c +++ b/daemon/external-helper.c @@ -37,20 +37,49 @@ POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include #include +static const int default_timout = 5; + /** * Call an external process */ -int run_external_process(const char *const *exec_args) +int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX], int tsec) { pid_t p; int status = 0; + int pipes[2]; + char internal[EXTERNAL_BUFFER_MAX] = { 0 }; + + if (pipe(pipes) == -1) { + LOG_ERROR("Could not create pipe: %s!\n", strerror(errno)); + return -1; + } + + /* Set the default timeout */ + if (tsec == -1) { + tsec = default_timout; + } + + /* set up our signaling for the child and the timout */ + sigset_t mask; + sigset_t omask; + sigemptyset(&mask); + sigaddset(&mask, SIGCHLD); + if (sigprocmask(SIG_BLOCK, &mask, &omask) < 0) { + LOG_ERROR("sigprocmask failed: %s\n", strerror(errno)); + return -1; + } if ((p = fork()) < 0) { LOG_ERROR("Failed to fork(): %s\n", strerror(errno)); return false; } else if (p == 0) { + /* Send STDOUT to the pipe */ + dup2(pipes[1], STDOUT_FILENO); + close(pipes[0]); + close(pipes[1]); /* Execute the command */ /* Note about cast: * The statement about argv[] and envp[] being constants is @@ -65,54 +94,30 @@ int run_external_process(const char *const *exec_args) _exit(EXIT_SUCCESS); } - if (waitpid(p, &status, 0) < 0) { - LOG_ERROR("Failed to waitpid(%d): %s\n", (int)p, strerror(errno)); - return -1; - } + /* Set up the timout */ + struct timespec timeout; + timeout.tv_sec = tsec; /* Magic timeout value of 5s for now - should be sane for most commands + */ + timeout.tv_nsec = 0; - /* i.e. sigsev */ - if (!WIFEXITED(status)) { - LOG_ERROR("Child process '%s' exited abnormally\n", exec_args[0]); - } else if (WEXITSTATUS(status) != 0) { - LOG_ERROR("External process failed\n"); - return -1; - } - - return 0; -} - -/** - * Call an external process and get output - */ -int run_external_process_get_output(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX]) -{ - pid_t p; - int status = 0; - int pipes[2]; - - if (pipe(pipes) == -1) { - LOG_ERROR("Could not create pipe: %s!\n", strerror(errno)); - return -1; - } - - if ((p = fork()) < 0) { - LOG_ERROR("Failed to fork(): %s\n", strerror(errno)); - return false; - } else if (p == 0) { - /* Send STDOUT to the pipe */ - dup2(pipes[1], STDOUT_FILENO); - close(pipes[0]); - close(pipes[1]); - /* Launch the process */ - if (execv(exec_args[0], (char *const *)exec_args) != 0) { - LOG_ERROR("Failed to execute external process %s: %s\n", exec_args[0], strerror(errno)); - exit(EXIT_FAILURE); + /* Wait for the child to finish up with a timout */ + while (true) { + if (sigtimedwait(&mask, NULL, &timeout) < 0) { + if (errno == EINTR) { + continue; + } else if (errno == EAGAIN) { + LOG_ERROR("Child process timed out for %s, killing and returning\n", exec_args[0]); + kill(p, SIGKILL); + } else { + LOG_ERROR("sigtimedwait failed: %s\n", strerror(errno)); + return -1; + } } - _exit(EXIT_SUCCESS); + break; } close(pipes[1]); - if (read(pipes[0], buffer, EXTERNAL_BUFFER_MAX) < 0) { + if (read(pipes[0], internal, EXTERNAL_BUFFER_MAX) < 0) { LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno)); return -1; } @@ -126,9 +131,13 @@ int run_external_process_get_output(const char *const *exec_args, char buffer[EX if (!WIFEXITED(status)) { LOG_ERROR("Child process '%s' exited abnormally\n", exec_args[0]); } else if (WEXITSTATUS(status) != 0) { - LOG_ERROR("External process failed\n"); + LOG_ERROR("External process failed with exit code %u\n", WEXITSTATUS(status)); + LOG_ERROR("Output was: %s\n", buffer ? buffer : internal); return -1; } + if (buffer) + memcpy(buffer, internal, EXTERNAL_BUFFER_MAX); + return 0; } diff --git a/daemon/external-helper.h b/daemon/external-helper.h index 0febe20..9d4e2e4 100644 --- a/daemon/external-helper.h +++ b/daemon/external-helper.h @@ -34,7 +34,4 @@ POSSIBILITY OF SUCH DAMAGE. #define EXTERNAL_BUFFER_MAX 1024 /* Run an external process and capture the return value */ -int run_external_process(const char *const *exec_args); - -/* Run an external process and capture the return value as well as output */ -int run_external_process_get_output(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX]); +int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX], int tsec); diff --git a/daemon/gamemode-gpu.c b/daemon/gamemode-gpu.c index 2ae3374..0040b9c 100644 --- a/daemon/gamemode-gpu.c +++ b/daemon/gamemode-gpu.c @@ -229,7 +229,7 @@ int game_mode_apply_gpu(const GameModeGPUInfo *info, bool apply) NULL, }; - if (run_external_process(exec_args) != 0) { + if (run_external_process(exec_args, NULL, -1) != 0) { LOG_ERROR("Failed to call gpuclockctl, could not apply optimisations!\n"); return -1; } @@ -262,7 +262,7 @@ int game_mode_get_gpu(GameModeGPUInfo *info) }; char buffer[EXTERNAL_BUFFER_MAX] = { 0 }; - if (run_external_process_get_output(exec_args, buffer) != 0) { + if (run_external_process(exec_args, buffer, -1) != 0) { LOG_ERROR("Failed to call gpuclockctl, could get values!\n"); return -1; } diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c index 08fdd04..2dd6c10 100644 --- a/daemon/gamemode-tests.c +++ b/daemon/gamemode-tests.c @@ -41,27 +41,45 @@ POSSIBILITY OF SUCH DAMAGE. #include #include "daemon_config.h" +#include "external-helper.h" #include "gamemode_client.h" #include "governors-query.h" #include "gpu-control.h" /* Initial verify step to ensure gamemode isn't already active */ -static int verify_gamemode_initial(void) +static int verify_gamemode_initial(struct GameModeConfig *config) { int status = 0; if ((status = gamemode_query_status()) != 0 && status != -1) { - LOG_ERROR("gamemode is currently active, tests require gamemode to start deactivated!\n"); - status = -1; + long reaper = config_get_reaper_frequency(config); + LOG_MSG("GameMode was active, waiting for the reaper thread (%ld seconds)!\n", reaper); + sleep(1); + + /* Try again after waiting */ + for (int i = 0; i < reaper; i++) { + if ((status = gamemode_query_status()) == 0) { + status = 0; + break; + } else if (status == -1) { + goto status_error; + } + LOG_MSG("Waiting...\n"); + sleep(1); + } + if (status == 1) + LOG_ERROR("GameMode still active, cannot run tests!\n"); } else if (status == -1) { - LOG_ERROR("gamemode_query_status failed: %s!\n", gamemode_error_string()); - LOG_ERROR("is gamemode installed correctly?\n"); - status = -1; + goto status_error; } else { status = 0; } return status; +status_error: + LOG_ERROR("gamemode_query_status failed: %s!\n", gamemode_error_string()); + LOG_ERROR("is gamemode installed correctly?\n"); + return -1; } /* Check if gamemode is active and this client is registered */ @@ -183,8 +201,8 @@ static int run_dual_client_tests(void) /* Parent process */ /* None of these should early-out as we need to clean up the child */ - /* Give the child a chance to reqeust gamemode */ - usleep(1000); + /* Give the child a chance to request gamemode */ + usleep(10000); /* Check that when we request gamemode, it replies that the other client is connected */ if (verify_other_client_connected() != 0) @@ -301,12 +319,11 @@ static int run_cpu_governor_tests(struct GameModeConfig *config) strcpy(desiredgov, "performance"); char defaultgov[CONFIG_VALUE_MAX] = { 0 }; - config_get_default_governor(config, defaultgov); - if (desiredgov[0] == '\0') { + if (defaultgov[0] == '\0') { const char *currentgov = get_gov_state(); if (currentgov) { - strncpy(desiredgov, currentgov, CONFIG_VALUE_MAX); + strncpy(defaultgov, currentgov, CONFIG_VALUE_MAX); } else { LOG_ERROR( "Could not get current CPU governor state, this indicates an error! See rest " @@ -342,6 +359,7 @@ static int run_cpu_governor_tests(struct GameModeConfig *config) static int run_custom_scripts_tests(struct GameModeConfig *config) { int scriptstatus = 0; + long timeout = config_get_script_timeout(config); /* Grab and test the start scripts */ char startscripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]; @@ -353,7 +371,8 @@ static int run_custom_scripts_tests(struct GameModeConfig *config) while (*startscripts[i] != '\0' && i < CONFIG_LIST_MAX) { LOG_MSG(":::: Running start script [%s]\n", startscripts[i]); - int ret = system(startscripts[i]); + const char *args[] = { "/bin/sh", "-c", startscripts[i], NULL }; + int ret = run_external_process(args, NULL, (int)timeout); if (ret == 0) LOG_MSG(":::: Passed\n"); @@ -375,7 +394,8 @@ static int run_custom_scripts_tests(struct GameModeConfig *config) while (*endscripts[i] != '\0' && i < CONFIG_LIST_MAX) { LOG_MSG(":::: Running end script [%s]\n", endscripts[i]); - int ret = system(endscripts[i]); + const char *args[] = { "/bin/sh", "-c", endscripts[i], NULL }; + int ret = run_external_process(args, NULL, (int)timeout); if (ret == 0) LOG_MSG(":::: Passed\n"); @@ -668,7 +688,7 @@ int game_mode_run_client_tests() /* First verify that gamemode is not currently active on the system * As well as it being currently installed and queryable */ - if (verify_gamemode_initial() != 0) + if (verify_gamemode_initial(config) != 0) return -1; /* Controls whether we require a supervisor to actually make requests */ diff --git a/daemon/gamemode.c b/daemon/gamemode.c index 6f206a2..1bce846 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -93,6 +93,7 @@ static void *game_mode_context_reaper(void *userdata); static void game_mode_context_enter(GameModeContext *self); static void game_mode_context_leave(GameModeContext *self); static char *game_mode_context_find_exe(pid_t pid); +static void game_mode_execute_scripts(char scripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX], int timeout); void game_mode_context_init(GameModeContext *self) { @@ -170,21 +171,6 @@ static void game_mode_context_enter(GameModeContext *self) LOG_MSG("Entering Game Mode...\n"); sd_notifyf(0, "STATUS=%sGameMode is now active.%s\n", "\x1B[1;32m", "\x1B[0m"); - char scripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]; - memset(scripts, 0, sizeof(scripts)); - config_get_gamemode_start_scripts(self->config, scripts); - - unsigned int i = 0; - while (*scripts[i] != '\0' && i < CONFIG_LIST_MAX) { - LOG_MSG("Executing script [%s]\n", scripts[i]); - int err; - if ((err = system(scripts[i])) != 0) { - /* Log the failure, but this is not fatal */ - LOG_ERROR("Script [%s] failed with error %d\n", scripts[i], err); - } - i++; - } - /* Read the initial governor state so we can revert it correctly */ const char *initial_state = get_gov_state(); if (initial_state) { @@ -203,7 +189,7 @@ static void game_mode_context_enter(GameModeContext *self) }; LOG_MSG("Requesting update of governor policy to %s\n", desiredGov); - if (run_external_process(exec_args) != 0) { + if (run_external_process(exec_args, NULL, -1) != 0) { LOG_ERROR("Failed to update cpu governor policy\n"); /* if the set fails, clear the initial mode so we don't try and reset it back and fail * again, presumably */ @@ -217,6 +203,14 @@ static void game_mode_context_enter(GameModeContext *self) /* Apply GPU optimisations */ game_mode_apply_gpu(self->gpu_info, true); + + /* Run custom scripts last - ensures the above are applied first and these scripts can react to + * them if needed */ + char scripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]; + memset(scripts, 0, sizeof(scripts)); + config_get_gamemode_start_scripts(self->config, scripts); + long timeout = config_get_script_timeout(self->config); + game_mode_execute_scripts(scripts, (int)timeout); } /** @@ -249,7 +243,7 @@ static void game_mode_context_leave(GameModeContext *self) }; LOG_MSG("Requesting update of governor policy to %s\n", gov_mode); - if (run_external_process(exec_args) != 0) { + if (run_external_process(exec_args, NULL, -1) != 0) { LOG_ERROR("Failed to update cpu governor policy\n"); } @@ -259,17 +253,8 @@ static void game_mode_context_leave(GameModeContext *self) char scripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]; memset(scripts, 0, sizeof(scripts)); config_get_gamemode_end_scripts(self->config, scripts); - - unsigned int i = 0; - while (*scripts[i] != '\0' && i < CONFIG_LIST_MAX) { - LOG_MSG("Executing script [%s]\n", scripts[i]); - int err; - if ((err = system(scripts[i])) != 0) { - /* Log the failure, but this is not fatal */ - LOG_ERROR("Script [%s] failed with error %d\n", scripts[i], err); - } - i++; - } + long timeout = config_get_script_timeout(self->config); + game_mode_execute_scripts(scripts, (int)timeout); } /** @@ -701,3 +686,19 @@ fail: LOG_ERROR("Unable to find executable for PID %d: %s\n", pid, strerror(errno)); return NULL; } + +/* Executes a set of scripts */ +static void game_mode_execute_scripts(char scripts[CONFIG_LIST_MAX][CONFIG_VALUE_MAX], int timeout) +{ + unsigned int i = 0; + while (*scripts[i] != '\0' && i < CONFIG_LIST_MAX) { + LOG_MSG("Executing script [%s]\n", scripts[i]); + int err; + const char *args[] = { "/bin/sh", "-c", scripts[i], NULL }; + if ((err = run_external_process(args, NULL, timeout)) != 0) { + /* Log the failure, but this is not fatal */ + LOG_ERROR("Script [%s] failed with error %d\n", scripts[i], err); + } + i++; + } +} diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c index 28963f4..001c07d 100644 --- a/daemon/gpuclockctl.c +++ b/daemon/gpuclockctl.c @@ -79,7 +79,7 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info) NV_CORE_OFFSET_ATTRIBUTE, info->nv_perf_level); const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; - if (run_external_process_get_output(exec_args_core, buf) != 0) { + if (run_external_process(exec_args_core, buf, -1) != 0) { LOG_ERROR("Failed to set %s!\n", arg); return -1; } @@ -98,7 +98,7 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info) NV_MEM_OFFSET_ATTRIBUTE, info->nv_perf_level); const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; - if (run_external_process_get_output(exec_args_mem, buf) != 0) { + if (run_external_process(exec_args_mem, buf, -1) != 0) { LOG_ERROR("Failed to set %s!\n", arg); return -1; } @@ -145,7 +145,7 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) info->nv_perf_level, info->core); const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-a", core_arg, NULL }; - if (run_external_process(exec_args_core) != 0) { + if (run_external_process(exec_args_core, NULL, -1) != 0) { LOG_ERROR("Failed to set %s!\n", core_arg); return -1; } @@ -160,7 +160,7 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) info->nv_perf_level, info->mem); const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-a", mem_arg, NULL }; - if (run_external_process(exec_args_mem) != 0) { + if (run_external_process(exec_args_mem, NULL, -1) != 0) { LOG_ERROR("Failed to set %s!\n", mem_arg); return -1; } diff --git a/lib/client_impl.c b/lib/client_impl.c index 4c23278..1627596 100644 --- a/lib/client_impl.c +++ b/lib/client_impl.c @@ -47,6 +47,8 @@ static int gamemode_request(const char *function, int arg) { sd_bus_message *msg = NULL; sd_bus *bus = NULL; + sd_bus_error err; + memset(&err, 0, sizeof(err)); int result = -1; @@ -64,7 +66,7 @@ static int gamemode_request(const char *function, int arg) "/com/feralinteractive/GameMode", "com.feralinteractive.GameMode", function, - NULL, + &err, &msg, arg ? "ii" : "i", getpid(), @@ -72,7 +74,13 @@ static int gamemode_request(const char *function, int arg) if (ret < 0) { snprintf(error_string, sizeof(error_string), - "Could not call method on bus: %s", + "Could not call method %s on com.feralinteractive.GameMode\n" + "\t%s\n" + "\t%s\n" + "\t%s\n", + function, + err.name, + err.message, strerror(-ret)); } else { // Read the reply