diff --git a/daemon/daemon_config.c b/daemon/daemon_config.c index 675c195..23e0e76 100644 --- a/daemon/daemon_config.c +++ b/daemon/daemon_config.c @@ -362,6 +362,7 @@ static void load_config_files(GameModeConfig *self) if (error) { LOG_MSG("Failed to parse config file - error on line %d!\n", error); } + fclose(f); } free(path); } diff --git a/daemon/external-helper.c b/daemon/external-helper.c index 52361f9..62cf29a 100644 --- a/daemon/external-helper.c +++ b/daemon/external-helper.c @@ -36,11 +36,75 @@ POSSIBILITY OF SUCH DAMAGE. #include #include +#include +#include #include -#include #include -static const int default_timeout = 5; +static const int DEFAULT_TIMEOUT = 5; + +static int read_child_stdout(int pipe_fd, char buffer[EXTERNAL_BUFFER_MAX], int tsec) +{ + fd_set fds; + struct timeval timeout; + int num_readable = 0; + ssize_t buffer_bytes_read = 0; + ssize_t just_read = 0; + bool buffer_full = false; + char discard_buffer[EXTERNAL_BUFFER_MAX]; + + /* Set up the timout */ + timeout.tv_sec = tsec; + timeout.tv_usec = 0; + + FD_ZERO(&fds); + + /* Wait for the child to finish up with a timout */ + while (true) { + FD_SET(pipe_fd, &fds); + num_readable = select(pipe_fd + 1, &fds, NULL, NULL, &timeout); + + if (num_readable < 0) { + if (errno == EINTR) { + continue; + } else { + LOG_ERROR("sigtimedwait failed: %s\n", strerror(errno)); + return -1; + } + } else if (num_readable == 0) { + return -2; + } + + if (!buffer_full) { + just_read = read(pipe_fd, + buffer + buffer_bytes_read, + EXTERNAL_BUFFER_MAX - (size_t)buffer_bytes_read - 1); + } else { + just_read = read(pipe_fd, discard_buffer, EXTERNAL_BUFFER_MAX - 1); + } + + if (just_read < 0) { + return -1; + } else if (just_read == 0) { + // EOF encountered + break; + } + + if (!buffer_full) { + buffer_bytes_read += just_read; + + if (buffer_bytes_read == EXTERNAL_BUFFER_MAX - 1) { + // our buffer is exhausted, discard the rest + // of the output + buffer_full = true; + } + } + } + + buffer[buffer_bytes_read] = 0; + + return 0; +} /** * Call an external process @@ -50,6 +114,7 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF pid_t p; int status = 0; int pipes[2]; + int ret = 0; char internal[EXTERNAL_BUFFER_MAX] = { 0 }; if (pipe(pipes) == -1) { @@ -59,20 +124,12 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF /* Set the default timeout */ if (tsec == -1) { - tsec = default_timeout; - } - - /* 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; + tsec = DEFAULT_TIMEOUT; } if ((p = fork()) < 0) { + close(pipes[0]); + close(pipes[1]); LOG_ERROR("Failed to fork(): %s\n", strerror(errno)); return false; } else if (p == 0) { @@ -91,38 +148,30 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF LOG_ERROR("Failed to execute external process: %s %s\n", exec_args[0], strerror(errno)); exit(EXIT_FAILURE); } - _exit(EXIT_SUCCESS); - } - - /* Set up the timout */ - struct timespec timeout; - timeout.tv_sec = tsec; - timeout.tv_nsec = 0; - - /* 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; - } - } - break; + // should never be reached + abort(); } + // close the write end of the pipe so we get signaled EOF once the + // child exits close(pipes[1]); - ssize_t output_size = read(pipes[0], internal, EXTERNAL_BUFFER_MAX - 1); - if (output_size < 0) { - LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno)); - return -1; - } + ret = read_child_stdout(pipes[0], internal, tsec); + close(pipes[0]); - internal[output_size] = 0; + if (ret != 0) { + if (ret == -2) { + LOG_ERROR("Child process timed out for %s, killing and returning\n", exec_args[0]); + kill(p, SIGKILL); + } else { + LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno)); + } + if (buffer) { + // make sure the buffer is a terminated empty string on error + buffer[0] = 0; + } + } else if (buffer) { + memcpy(buffer, internal, EXTERNAL_BUFFER_MAX); + } if (waitpid(p, &status, 0) < 0) { LOG_ERROR("Failed to waitpid(%d): %s\n", (int)p, strerror(errno)); @@ -134,12 +183,9 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF LOG_ERROR("Child process '%s' exited abnormally\n", exec_args[0]); } else if (WEXITSTATUS(status) != 0) { LOG_ERROR("External process failed with exit code %d\n", WEXITSTATUS(status)); - LOG_ERROR("Output was: %s\n", buffer ? buffer : internal); + LOG_ERROR("Output was: %s\n", internal); return -1; } - if (buffer) - memcpy(buffer, internal, EXTERNAL_BUFFER_MAX); - return 0; } diff --git a/daemon/governors-query.c b/daemon/governors-query.c index 9e0af0f..9d483c0 100644 --- a/daemon/governors-query.c +++ b/daemon/governors-query.c @@ -133,6 +133,7 @@ const char *get_gov_state(void) /* Don't handle the mixed case, this shouldn't ever happen * But it is a clear sign we shouldn't carry on */ LOG_ERROR("Governors malformed: got \"%s\", expected \"%s\"", contents, governor); + fclose(f); return "malformed"; } diff --git a/daemon/gpu-control.c b/daemon/gpu-control.c index 147cdd1..a2699c0 100644 --- a/daemon/gpu-control.c +++ b/daemon/gpu-control.c @@ -50,7 +50,10 @@ enum GPUVendor gamemode_get_gpu_vendor(long device) return Vendor_Invalid; } char buff[64]; - if (fgets(buff, 64, file) != NULL) { + bool got_line = fgets(buff, 64, file) != NULL; + fclose(file); + + if (got_line) { vendor = strtol(buff, NULL, 0); } else { LOG_ERROR("Coudn't read contents of file %s, will not apply optimisations!\n", path); diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c index 73c17c5..850bd05 100644 --- a/daemon/gpuclockctl.c +++ b/daemon/gpuclockctl.c @@ -36,6 +36,8 @@ POSSIBILITY OF SUCH DAMAGE. #include "external-helper.h" #include "gpu-control.h" +#include + /* NV constants */ #define NV_CORE_OFFSET_ATTRIBUTE "GPUGraphicsClockOffset" #define NV_MEM_OFFSET_ATTRIBUTE "GPUMemoryTransferRateOffset" @@ -44,6 +46,7 @@ POSSIBILITY OF SUCH DAMAGE. #define NV_PCIDEVICE_ATTRIBUTE "PCIDevice" #define NV_ATTRIBUTE_FORMAT "[gpu:%ld]/%s" #define NV_PERF_LEVEL_FORMAT "[%ld]" +#define NV_ARG_MAX 128 /* AMD constants */ #define AMD_DRM_PATH "/sys/class/drm/card%ld/device/%s" @@ -64,6 +67,30 @@ static void print_usage_and_exit(void) exit(EXIT_FAILURE); } +static const char *get_nv_attr(const char *attr) +{ + static char out[EXTERNAL_BUFFER_MAX]; + const char *exec_args[] = { "/usr/bin/nvidia-settings", "-q", attr, "-t", NULL }; + if (run_external_process(exec_args, out, -1) != 0) { + LOG_ERROR("Failed to get %s!\n", attr); + out[0] = 0; + return NULL; + } + + return &out[0]; +} + +static int set_nv_attr(const char *attr) +{ + const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-a", attr, NULL }; + if (run_external_process(exec_args_core, NULL, -1) != 0) { + LOG_ERROR("Failed to set %s!\n", attr); + return -1; + } + + return 0; +} + /* Get the nvidia driver index for the current GPU */ static long get_gpu_index_id_nv(struct GameModeGPUInfo *info) { @@ -108,23 +135,21 @@ static long get_max_perf_level_nv(struct GameModeGPUInfo *info) if (!getenv("DISPLAY")) LOG_ERROR("Getting Nvidia parameters requires DISPLAY to be set - will likely fail!\n"); - char arg[128] = { 0 }; - char buf[EXTERNAL_BUFFER_MAX] = { 0 }; + char arg[NV_ARG_MAX] = { 0 }; + const char *attr; - snprintf(arg, 128, NV_ATTRIBUTE_FORMAT, info->device, NV_PERFMODES_ATTRIBUTE); - const char *exec_args[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; - if (run_external_process(exec_args, buf, -1) != 0) { - LOG_ERROR("Failed to get %s!\n", arg); + snprintf(arg, NV_ARG_MAX, NV_ATTRIBUTE_FORMAT, info->device, NV_PERFMODES_ATTRIBUTE); + if ((attr = get_nv_attr(arg)) == NULL) { return -1; } - char *ptr = strrchr(buf, ';'); + char *ptr = strrchr(attr, ';'); long level = -1; if (!ptr || sscanf(ptr, "; perf=%ld", &level) != 1) { LOG_ERROR( "Output didn't match expected format, couldn't discern highest perf level from " "nvidia-settings!\n"); - LOG_ERROR("Output:%s\n", buf); + LOG_ERROR("Output:%s\n", attr); return -1; } @@ -142,59 +167,53 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info) long perf_level = get_max_perf_level_nv(info); - char arg[128] = { 0 }; - char buf[EXTERNAL_BUFFER_MAX] = { 0 }; + char arg[NV_ARG_MAX] = { 0 }; + const char *attr; char *end; /* Get the GPUGraphicsClockOffset parameter */ snprintf(arg, - 128, + NV_ARG_MAX, NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT, info->device, NV_CORE_OFFSET_ATTRIBUTE, perf_level); - const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; - if (run_external_process(exec_args_core, buf, -1) != 0) { - LOG_ERROR("Failed to get %s!\n", arg); + if ((attr = get_nv_attr(arg)) == NULL) { return -1; } - info->nv_core = strtol(buf, &end, 10); - if (end == buf) { - LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf); + info->nv_core = strtol(attr, &end, 10); + if (end == attr) { + LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, attr); return -1; } /* Get the GPUMemoryTransferRateOffset parameter */ snprintf(arg, - 128, + NV_ARG_MAX, NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT, info->device, NV_MEM_OFFSET_ATTRIBUTE, perf_level); - const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; - if (run_external_process(exec_args_mem, buf, -1) != 0) { - LOG_ERROR("Failed to get %s!\n", arg); + if ((attr = get_nv_attr(arg)) == NULL) { return -1; } - info->nv_mem = strtol(buf, &end, 10); - if (end == buf) { - LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf); + info->nv_mem = strtol(attr, &end, 10); + if (end == attr) { + LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, attr); return -1; } /* Get the GPUPowerMizerMode parameter */ - snprintf(arg, 128, NV_ATTRIBUTE_FORMAT, info->device, NV_POWERMIZER_MODE_ATTRIBUTE); - const char *exec_args_pm[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; - if (run_external_process(exec_args_pm, buf, -1) != 0) { - LOG_ERROR("Failed to get %s!\n", arg); + snprintf(arg, NV_ARG_MAX, NV_ATTRIBUTE_FORMAT, info->device, NV_POWERMIZER_MODE_ATTRIBUTE); + if ((attr = get_nv_attr(arg)) == NULL) { return -1; } - info->nv_powermizer_mode = strtol(buf, &end, 10); - if (end == buf) { - LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf); + info->nv_powermizer_mode = strtol(attr, &end, 10); + if (end == attr) { + LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, attr); return -1; } @@ -218,20 +237,18 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) long perf_level = get_max_perf_level_nv(info); - char arg[128] = { 0 }; + char arg[NV_ARG_MAX] = { 0 }; /* Set the GPUGraphicsClockOffset parameter */ if (info->nv_core != -1) { snprintf(arg, - 128, + NV_ARG_MAX, NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT "=%ld", info->device, NV_CORE_OFFSET_ATTRIBUTE, perf_level, info->nv_core); - const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-a", arg, NULL }; - if (run_external_process(exec_args_core, NULL, -1) != 0) { - LOG_ERROR("Failed to set %s!\n", arg); + if (set_nv_attr(arg) != 0) { status = -1; } } @@ -239,15 +256,13 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) /* Set the GPUMemoryTransferRateOffset parameter */ if (info->nv_mem != -1) { snprintf(arg, - 128, + NV_ARG_MAX, NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT "=%ld", info->device, NV_MEM_OFFSET_ATTRIBUTE, perf_level, info->nv_mem); - const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-a", arg, NULL }; - if (run_external_process(exec_args_mem, NULL, -1) != 0) { - LOG_ERROR("Failed to set %s!\n", arg); + if (set_nv_attr(arg) != 0) { status = -1; } } @@ -255,14 +270,12 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) /* Set the GPUPowerMizerMode parameter if requested */ if (info->nv_powermizer_mode != -1) { snprintf(arg, - 128, + NV_ARG_MAX, NV_ATTRIBUTE_FORMAT "=%ld", info->device, NV_POWERMIZER_MODE_ATTRIBUTE, info->nv_powermizer_mode); - const char *exec_args_pm[] = { "/usr/bin/nvidia-settings", "-a", arg, NULL }; - if (run_external_process(exec_args_pm, NULL, -1) != 0) { - LOG_ERROR("Failed to set %s!\n", arg); + if (set_nv_attr(arg) != 0) { status = -1; } } @@ -280,8 +293,8 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) return -1; /* Get the contents of power_dpm_force_performance_level */ - char path[64]; - snprintf(path, 64, AMD_DRM_PATH, info->device, "power_dpm_force_performance_level"); + char path[PATH_MAX]; + snprintf(path, PATH_MAX, AMD_DRM_PATH, info->device, "power_dpm_force_performance_level"); FILE *file = fopen(path, "r"); if (!file) { @@ -289,21 +302,26 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) return -1; } + int ret = 0; + char buff[CONFIG_VALUE_MAX] = { 0 }; if (!fgets(buff, CONFIG_VALUE_MAX, file)) { LOG_ERROR("Could not read file %s (%s)!\n", path, strerror(errno)); - return -1; + ret = -1; } if (fclose(file) != 0) { LOG_ERROR("Could not close %s after reading (%s)!\n", path, strerror(errno)); - return -1; + ret = -1; } - /* Copy in the value from the file */ - strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX - 1); - info->amd_performance_level[CONFIG_VALUE_MAX - 1] = '\0'; - return info == NULL; + if (ret == 0) { + /* Copy in the value from the file */ + strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX - 1); + info->amd_performance_level[CONFIG_VALUE_MAX - 1] = '\0'; + } + + return ret; } /* @@ -311,8 +329,8 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) */ static int set_gpu_state_amd_file(const char *filename, long device, const char *value) { - char path[64]; - snprintf(path, 64, AMD_DRM_PATH, device, filename); + char path[PATH_MAX]; + snprintf(path, PATH_MAX, AMD_DRM_PATH, device, filename); FILE *file = fopen(path, "w"); if (!file) { @@ -320,17 +338,19 @@ static int set_gpu_state_amd_file(const char *filename, long device, const char return -1; } + int ret = 0; + if (fprintf(file, "%s", value) < 0) { LOG_ERROR("Could not write to %s (%s)!\n", path, strerror(errno)); - return -1; + ret = -1; } if (fclose(file) != 0) { LOG_ERROR("Could not close %s after writing (%s)!\n", path, strerror(errno)); - return -1; + ret = -1; } - return 0; + return ret; } /** @@ -390,10 +410,11 @@ static long get_generic_value(const char *val) */ int main(int argc, char *argv[]) { + struct GameModeGPUInfo info; + memset(&info, 0, sizeof(info)); + if (argc == 3 && strncmp(argv[2], "get", 3) == 0) { /* Get and verify the vendor and device */ - struct GameModeGPUInfo info; - memset(&info, 0, sizeof(info)); info.device = get_device(argv[1]); info.vendor = gamemode_get_gpu_vendor(info.device); @@ -420,8 +441,6 @@ int main(int argc, char *argv[]) } else if (argc >= 4 && argc <= 7 && strncmp(argv[2], "set", 3) == 0) { /* Get and verify the vendor and device */ - struct GameModeGPUInfo info; - memset(&info, 0, sizeof(info)); info.device = get_device(argv[1]); info.vendor = gamemode_get_gpu_vendor(info.device);