From c4ff3d7250d79929e16de36d17e48b2efcf896fc Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Mon, 8 Oct 2018 19:29:24 +0200 Subject: [PATCH 1/9] refactor: Break helpers out of the main daemon source Signed-off-by: Kai Krakow --- daemon/gamemode.c | 29 +-------------------- daemon/helpers.h | 64 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 28 deletions(-) create mode 100644 daemon/helpers.h diff --git a/daemon/gamemode.c b/daemon/gamemode.c index 047ccca..b89219c 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -35,6 +35,7 @@ POSSIBILITY OF SUCH DAMAGE. #include "daemon_config.h" #include "governors-query.h" #include "governors.h" +#include "helpers.h" #include "ioprio.h" #include "logging.h" @@ -47,9 +48,6 @@ POSSIBILITY OF SUCH DAMAGE. #include #include #include -#include -#include -#include #include #include #include @@ -66,31 +64,6 @@ POSSIBILITY OF SUCH DAMAGE. */ #define NICE_DEFAULT_PRIORITY -4 -/* Value clamping helper. - */ -#define CLAMP(lbound, ubound, value) MIN(MIN(lbound, ubound), MAX(MAX(lbound, ubound), value)) - -/* Little helper to safely print into a buffer, returns a pointer into the buffer - */ -#define buffered_snprintf(b, s, ...) \ - (snprintf(b, sizeof(b), s, __VA_ARGS__) < (ssize_t)sizeof(b) ? b : NULL) - -/* Little helper to safely print into a buffer, returns a newly allocated string - */ -#define safe_snprintf(b, s, ...) \ - (snprintf(b, sizeof(b), s, __VA_ARGS__) < (ssize_t)sizeof(b) ? strndup(b, sizeof(b)) : NULL) - -/** - * Helper function: Test, if haystack ends with needle. - */ -static inline const char *strtail(const char *haystack, const char *needle) -{ - char *pos = strstr(haystack, needle); - if (pos && (strlen(pos) == strlen(needle))) - return pos; - return NULL; -} - /** * The GameModeClient encapsulates the remote connection, providing a list * form to contain the pid and credentials. diff --git a/daemon/helpers.h b/daemon/helpers.h new file mode 100644 index 0000000..6b0fff6 --- /dev/null +++ b/daemon/helpers.h @@ -0,0 +1,64 @@ +/* + +Copyright (c) 2017-2018, Feral Interactive +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + * Neither the name of Feral Interactive nor the names of its contributors + may be used to endorse or promote products derived from this software + without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +POSSIBILITY OF SUCH DAMAGE. + + */ + +#pragma once + +#include +#include +#include + +/** + * 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)) + +/** + * Little helper to safely print into a buffer, returns a pointer into the buffer + */ +#define buffered_snprintf(b, s, ...) \ + (snprintf(b, sizeof(b), s, __VA_ARGS__) < (ssize_t)sizeof(b) ? b : NULL) + +/** + * Little helper to safely print into a buffer, returns a newly allocated string + */ +#define safe_snprintf(b, s, ...) \ + (snprintf(b, sizeof(b), s, __VA_ARGS__) < (ssize_t)sizeof(b) ? strndup(b, sizeof(b)) : NULL) + +/** + * Helper function: Test, if haystack ends with needle. + */ +static inline const char *strtail(const char *haystack, const char *needle) +{ + char *pos = strstr(haystack, needle); + if (pos && (strlen(pos) == strlen(needle))) + return pos; + return NULL; +} From 748808be7ed0db146c18fa39c54c42a3d99f3867 Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Mon, 8 Oct 2018 20:29:12 +0200 Subject: [PATCH 2/9] refactor: Break proc API functions out of the main daemon source Signed-off-by: Kai Krakow --- daemon/gamemode-proc.c | 61 ++++++++++++++++++++++++++++++++++++++++++ daemon/gamemode.c | 19 ++++--------- daemon/gamemode.h | 11 ++++++++ daemon/meson.build | 1 + 4 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 daemon/gamemode-proc.c diff --git a/daemon/gamemode-proc.c b/daemon/gamemode-proc.c new file mode 100644 index 0000000..74d083d --- /dev/null +++ b/daemon/gamemode-proc.c @@ -0,0 +1,61 @@ +/* + +Copyright (c) 2017-2018, Feral Interactive +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + * Neither the name of Feral Interactive nor the names of its contributors + may be used to endorse or promote products derived from this software + without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +POSSIBILITY OF SUCH DAMAGE. + + */ + +#define _GNU_SOURCE + +#include "gamemode.h" +#include "helpers.h" + +#include +#include +#include + +/** + * Opens the process environment for a specific PID and returns + * a file descriptor to the directory /proc/PID. Doing it that way prevents + * the directory going MIA when a process exits while we are looking at it + * and allows us to handle fewer error cases. + */ +procfd_t game_mode_open_proc(const pid_t pid) +{ + char buffer[PATH_MAX]; + const char *proc_path = buffered_snprintf(buffer, "/proc/%d", pid); + + return proc_path ? open(proc_path, O_RDONLY | O_CLOEXEC) : INVALID_PROCFD; +} + +/** + * Closes the process environment. + */ +int game_mode_close_proc(const procfd_t procfd) +{ + return close(procfd); +} diff --git a/daemon/gamemode.c b/daemon/gamemode.c index b89219c..e2578a2 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -41,7 +41,6 @@ POSSIBILITY OF SUCH DAMAGE. #include #include -#include #include #include #include @@ -50,7 +49,6 @@ POSSIBILITY OF SUCH DAMAGE. #include #include #include -#include #include /* SCHED_ISO may not be defined as it is a reserved value not yet @@ -725,7 +723,7 @@ GameModeContext *game_mode_context_instance() * Lookup the process environment for a specific variable or return NULL. * Requires an open directory FD from /proc/PID. */ -static char *game_mode_lookup_proc_env(int proc_fd, const char *var) +static char *game_mode_lookup_proc_env(const procfd_t proc_fd, const char *var) { char *environ = NULL; @@ -785,18 +783,11 @@ static char *game_mode_resolve_wine_preloader(pid_t pid) { char buffer[PATH_MAX]; char *proc_path = NULL, *wine_exe = NULL, *wineprefix = NULL; - int proc_fd = -1; - - /* We could use the buffered_snprintf() helper here but it may potentially - * overwrite proc_path when the buffer is re-used later and usage of - * proc_path has not been discarded yet (i.e., it's used in the fail path). - * Let's not introduce this non-obvious pitfall. - */ - if (!(proc_path = safe_snprintf(buffer, "/proc/%d", pid))) - goto fail; /* Open the directory, we are potentially reading multiple files from it */ - if (-1 == (proc_fd = open(proc_path, O_RDONLY | O_CLOEXEC))) + procfd_t proc_fd = game_mode_open_proc(pid); + + if (proc_fd == INVALID_PROCFD) goto fail_proc; /* Open the command line */ @@ -868,7 +859,7 @@ static char *game_mode_resolve_wine_preloader(pid_t pid) goto fail; error_cleanup: - close(proc_fd); + game_mode_close_proc(proc_fd); free(wineprefix); free(proc_path); return wine_exe; diff --git a/daemon/gamemode.h b/daemon/gamemode.h index 681f50a..81f68b7 100644 --- a/daemon/gamemode.h +++ b/daemon/gamemode.h @@ -34,6 +34,10 @@ POSSIBILITY OF SUCH DAMAGE. #include #include +#define INVALID_PROCFD -1 + +typedef int procfd_t; + /** * Opaque context */ @@ -83,3 +87,10 @@ bool game_mode_context_unregister(GameModeContext *self, pid_t pid); * 2 if gamemode is active and the client is registered */ int game_mode_context_query_status(GameModeContext *self, pid_t pid); + +/** gamemode-proc.c + * Provides internal API functions specific to working with process + * environments. + */ +procfd_t game_mode_open_proc(const pid_t pid); +int game_mode_close_proc(const procfd_t procfd); diff --git a/daemon/meson.build b/daemon/meson.build index 9f5f280..34d8caf 100644 --- a/daemon/meson.build +++ b/daemon/meson.build @@ -18,6 +18,7 @@ link_daemon_common = declare_dependency( daemon_sources = [ 'main.c', 'gamemode.c', + 'gamemode-proc.c', 'daemonize.c', 'dbus_messaging.c', 'governors.c', From 92967b135b28d413dc4b4d7a6eeaa9502c6ed4c9 Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Mon, 8 Oct 2018 20:57:25 +0200 Subject: [PATCH 3/9] refactor: Break env API functions out of the main daemon source Signed-off-by: Kai Krakow --- daemon/gamemode-env.c | 96 +++++++++++++++++++++++++++++++++++++++++++ daemon/gamemode.c | 56 ------------------------- daemon/gamemode.h | 7 ++++ daemon/meson.build | 1 + 4 files changed, 104 insertions(+), 56 deletions(-) create mode 100644 daemon/gamemode-env.c diff --git a/daemon/gamemode-env.c b/daemon/gamemode-env.c new file mode 100644 index 0000000..655df33 --- /dev/null +++ b/daemon/gamemode-env.c @@ -0,0 +1,96 @@ +/* + +Copyright (c) 2017-2018, Feral Interactive +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + * Neither the name of Feral Interactive nor the names of its contributors + may be used to endorse or promote products derived from this software + without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +POSSIBILITY OF SUCH DAMAGE. + + */ + +#define _GNU_SOURCE + +#include "gamemode.h" + +#include +#include +#include +#include +#include +#include + +/** + * Lookup the process environment for a specific variable or return NULL. + * Requires an open directory FD from /proc/PID. + */ +char *game_mode_lookup_proc_env(const procfd_t proc_fd, const char *var) +{ + char *environ = NULL; + + int fd = openat(proc_fd, "environ", O_RDONLY | O_CLOEXEC); + if (fd != -1) { + FILE *stream = fdopen(fd, "r"); + if (stream) { + /* Read every \0 terminated line from the environment */ + char *line = NULL; + size_t len = 0; + size_t pos = strlen(var) + 1; + while (!environ && (getdelim(&line, &len, 0, stream) != -1)) { + /* Find a match including the "=" suffix */ + if ((len > pos) && (strncmp(line, var, strlen(var)) == 0) && (line[pos - 1] == '=')) + environ = strndup(line + pos, len - pos); + } + free(line); + fclose(stream); + } else + close(fd); + } + + /* If found variable is empty, skip it */ + if (environ && !strlen(environ)) { + free(environ); + environ = NULL; + } + + return environ; +} + +/** + * Lookup the home directory of the user in a safe way. + */ +char *game_mode_lookup_user_home(void) +{ + /* Try loading env HOME first */ + const char *home = secure_getenv("HOME"); + if (!home) { + /* If HOME is not defined (or out of context), fall back to passwd */ + struct passwd *pw = getpwuid(getuid()); + if (!pw) + return NULL; + home = pw->pw_dir; + } + + /* Try to allocate into our heap */ + return home ? strdup(home) : NULL; +} diff --git a/daemon/gamemode.c b/daemon/gamemode.c index e2578a2..1f550d2 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -43,7 +43,6 @@ POSSIBILITY OF SUCH DAMAGE. #include #include #include -#include #include #include #include @@ -719,61 +718,6 @@ GameModeContext *game_mode_context_instance() return &instance; } -/** - * Lookup the process environment for a specific variable or return NULL. - * Requires an open directory FD from /proc/PID. - */ -static char *game_mode_lookup_proc_env(const procfd_t proc_fd, const char *var) -{ - char *environ = NULL; - - int fd = openat(proc_fd, "environ", O_RDONLY | O_CLOEXEC); - if (fd != -1) { - FILE *stream = fdopen(fd, "r"); - if (stream) { - /* Read every \0 terminated line from the environment */ - char *line = NULL; - size_t len = 0; - size_t pos = strlen(var) + 1; - while (!environ && (getdelim(&line, &len, 0, stream) != -1)) { - /* Find a match including the "=" suffix */ - if ((len > pos) && (strncmp(line, var, strlen(var)) == 0) && (line[pos - 1] == '=')) - environ = strndup(line + pos, len - pos); - } - free(line); - fclose(stream); - } else - close(fd); - } - - /* If found variable is empty, skip it */ - if (environ && !strlen(environ)) { - free(environ); - environ = NULL; - } - - return environ; -} - -/** - * Lookup the home directory of the user in a safe way. - */ -static char *game_mode_lookup_user_home(void) -{ - /* Try loading env HOME first */ - const char *home = secure_getenv("HOME"); - if (!home) { - /* If HOME is not defined (or out of context), fall back to passwd */ - struct passwd *pw = getpwuid(getuid()); - if (!pw) - return NULL; - home = pw->pw_dir; - } - - /* Try to allocate into our heap */ - return home ? strdup(home) : NULL; -} - /** * Attempt to resolve the exe for wine-preloader. * This function is used if game_mode_context_find_exe() identified the diff --git a/daemon/gamemode.h b/daemon/gamemode.h index 81f68b7..f9df524 100644 --- a/daemon/gamemode.h +++ b/daemon/gamemode.h @@ -88,6 +88,13 @@ bool game_mode_context_unregister(GameModeContext *self, pid_t pid); */ int game_mode_context_query_status(GameModeContext *self, pid_t pid); +/** gamemode-env.c + * Provides internal API functions specific to working environment + * variables. + */ +char *game_mode_lookup_proc_env(const procfd_t proc_fd, const char *var); +char *game_mode_lookup_user_home(void); + /** gamemode-proc.c * Provides internal API functions specific to working with process * environments. diff --git a/daemon/meson.build b/daemon/meson.build index 34d8caf..5f4dcc2 100644 --- a/daemon/meson.build +++ b/daemon/meson.build @@ -18,6 +18,7 @@ link_daemon_common = declare_dependency( daemon_sources = [ 'main.c', 'gamemode.c', + 'gamemode-env.c', 'gamemode-proc.c', 'daemonize.c', 'dbus_messaging.c', From b9c9a5f12044a91126fb85320e9d4627664bdd12 Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Mon, 8 Oct 2018 21:45:23 +0200 Subject: [PATCH 4/9] refactor: Avoid exposing various internal types Signed-off-by: Kai Krakow --- daemon/gamemode.c | 5 +++++ daemon/gamemode.h | 11 ++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/daemon/gamemode.c b/daemon/gamemode.c index 1f550d2..3d4a1e7 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -718,6 +718,11 @@ GameModeContext *game_mode_context_instance() return &instance; } +GameModeConfig *game_mode_config_from_context(const GameModeContext *context) +{ + return context ? context->config : NULL; +} + /** * Attempt to resolve the exe for wine-preloader. * This function is used if game_mode_context_find_exe() identified the diff --git a/daemon/gamemode.h b/daemon/gamemode.h index f9df524..201cf85 100644 --- a/daemon/gamemode.h +++ b/daemon/gamemode.h @@ -39,9 +39,10 @@ POSSIBILITY OF SUCH DAMAGE. typedef int procfd_t; /** - * Opaque context + * Opaque types */ typedef struct GameModeContext GameModeContext; +typedef struct GameModeConfig GameModeConfig; /** * Return the singleton instance @@ -88,6 +89,14 @@ bool game_mode_context_unregister(GameModeContext *self, pid_t pid); */ int game_mode_context_query_status(GameModeContext *self, pid_t pid); +/** + * Query the config of a gamemode context + * + * @param context A gamemode context + * @returns Configuration from the gamemode context + */ +GameModeConfig *game_mode_config_from_context(const GameModeContext *context); + /** gamemode-env.c * Provides internal API functions specific to working environment * variables. From edf9257de43d9774ae797858f4bdeb3d8db6b5d1 Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Mon, 8 Oct 2018 21:49:28 +0200 Subject: [PATCH 5/9] refactor: Break sched API functions out of the main daemon source Signed-off-by: Kai Krakow --- daemon/gamemode-sched.c | 157 ++++++++++++++++++++++++++++++++++++++++ daemon/gamemode.c | 114 +---------------------------- daemon/gamemode.h | 7 ++ daemon/meson.build | 1 + 4 files changed, 167 insertions(+), 112 deletions(-) create mode 100644 daemon/gamemode-sched.c diff --git a/daemon/gamemode-sched.c b/daemon/gamemode-sched.c new file mode 100644 index 0000000..cd2a2ed --- /dev/null +++ b/daemon/gamemode-sched.c @@ -0,0 +1,157 @@ +/* + +Copyright (c) 2017-2018, Feral Interactive +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + * Neither the name of Feral Interactive nor the names of its contributors + may be used to endorse or promote products derived from this software + without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +POSSIBILITY OF SUCH DAMAGE. + + */ + +#define _GNU_SOURCE + +#include "daemon_config.h" +#include "gamemode.h" +#include "logging.h" + +#include +#include +#include +#include +#include + +/** + * Priority to renice the process to. + */ +#define NICE_DEFAULT_PRIORITY -4 + +/** + * Apply scheduling policies + * + * 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) +{ + GameModeConfig *config = game_mode_config_from_context(self); + + /* + * read configuration "renice" (1..20) + */ + long int renice = 0; + config_get_renice_value(config, &renice); + if ((renice < 1) || (renice > 20)) { + LOG_ERROR("Invalid renice value '%ld' reset to default: %d.\n", + renice, + -NICE_DEFAULT_PRIORITY); + renice = NICE_DEFAULT_PRIORITY; + } 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); + } else if (setpriority(PRIO_PROCESS, (id_t)client, (int)renice)) { + LOG_ERROR( + "Failed to renice client [%d], ignoring error condition: %s\n" + " -- Your user may not have permission to do this. Please read the docs\n" + " -- to learn how to adjust the pam limits.\n", + client, + strerror(errno)); + } +} + +void game_mode_apply_scheduling(const GameModeContext *self, const pid_t client) +{ + GameModeConfig *config = game_mode_config_from_context(self); + + /* + * read configuration "softrealtime" (on, off, auto) + */ + char softrealtime[CONFIG_VALUE_MAX] = { 0 }; + config_get_soft_realtime(config, softrealtime); + + /* + * Enable unconditionally or auto-detect soft realtime usage, + * auto detection is based on observations where dual-core CPU suffered + * priority inversion problems with the graphics driver thus running + * slower as a result, so enable only with more than 3 cores. + */ + bool enable_softrealtime = (strcmp(softrealtime, "on") == 0) || (get_nprocs() > 3); + + /* + * Actually apply the scheduler policy if not explicitly turned off + */ + if (!(strcmp(softrealtime, "off") == 0) && (enable_softrealtime)) { + const struct sched_param p = { .sched_priority = 0 }; + if (sched_setscheduler(client, SCHED_ISO | SCHED_RESET_ON_FORK, &p)) { + const char *additional_message = ""; + switch (errno) { + case EPERM: { + static int once = 0; + if (once++) + break; + additional_message = + " -- The error indicates that you may be running a resource management\n" + " -- daemon managing your game launcher and it leaks lower scheduling\n" + " -- classes into the games. This is likely a bug in the management daemon\n" + " -- and not a bug in GameMode, it should be reported upstream.\n" + " -- If unsure, please also look here:\n" + " -- https://github.com/FeralInteractive/gamemode/issues/68\n"; + break; + } + case EINVAL: { + static int once = 0; + if (once++) + break; + additional_message = + " -- The error indicates that your kernel may not support this. If you\n" + " -- don't know what SCHED_ISO means, you can safely ignore this. If you\n" + " -- expected it to work, ensure you're running a kernel with MuQSS or\n" + " -- PDS scheduler.\n" + " -- For further technical reading on the topic start here:\n" + " -- https://lwn.net/Articles/720227/\n"; + break; + } + } + LOG_ERROR( + "Failed setting client [%d] into SCHED_ISO mode, ignoring error condition: %s\n%s", + client, + strerror(errno), + additional_message); + } + } else { + LOG_ERROR("Skipped setting client [%d] into SCHED_ISO mode: softrealtime setting is '%s'\n", + client, + softrealtime); + } +} diff --git a/daemon/gamemode.c b/daemon/gamemode.c index 3d4a1e7..79da96c 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -41,13 +41,9 @@ POSSIBILITY OF SUCH DAMAGE. #include #include -#include #include -#include #include #include -#include -#include #include /* SCHED_ISO may not be defined as it is a reserved value not yet @@ -57,10 +53,6 @@ POSSIBILITY OF SUCH DAMAGE. #define SCHED_ISO 4 #endif -/* Priority to renice the process to. - */ -#define NICE_DEFAULT_PRIORITY -4 - /** * The GameModeClient encapsulates the remote connection, providing a list * form to contain the pid and credentials. @@ -167,109 +159,6 @@ void game_mode_context_destroy(GameModeContext *self) pthread_rwlock_destroy(&self->rwlock); } -/** - * Apply scheduling policies - * - * 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. - */ -static void game_mode_apply_scheduler(GameModeContext *self, pid_t client) -{ - /* - * read configuration "renice" (1..20) - */ - long int renice = 0; - config_get_renice_value(self->config, &renice); - if ((renice < 1) || (renice > 20)) { - LOG_ERROR("Invalid renice value '%ld' reset to default: %d.\n", - renice, - -NICE_DEFAULT_PRIORITY); - renice = NICE_DEFAULT_PRIORITY; - } 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); - } else if (setpriority(PRIO_PROCESS, (id_t)client, (int)renice)) { - LOG_ERROR( - "Failed to renice client [%d], ignoring error condition: %s\n" - " -- Your user may not have permission to do this. Please read the docs\n" - " -- to learn how to adjust the pam limits.\n", - client, - strerror(errno)); - } - - /* - * read configuration "softrealtime" (on, off, auto) - */ - char softrealtime[CONFIG_VALUE_MAX] = { 0 }; - config_get_soft_realtime(self->config, softrealtime); - - /* - * Enable unconditionally or auto-detect soft realtime usage, - * auto detection is based on observations where dual-core CPU suffered - * priority inversion problems with the graphics driver thus running - * slower as a result, so enable only with more than 3 cores. - */ - bool enable_softrealtime = (strcmp(softrealtime, "on") == 0) || (get_nprocs() > 3); - - /* - * Actually apply the scheduler policy if not explicitly turned off - */ - if (!(strcmp(softrealtime, "off") == 0) && (enable_softrealtime)) { - const struct sched_param p = { .sched_priority = 0 }; - if (sched_setscheduler(client, SCHED_ISO | SCHED_RESET_ON_FORK, &p)) { - const char *additional_message = ""; - switch (errno) { - case EPERM: { - static int once = 0; - if (once++) - break; - additional_message = - " -- The error indicates that you may be running a resource management\n" - " -- daemon managing your game launcher and it leaks lower scheduling\n" - " -- classes into the games. This is likely a bug in the management daemon\n" - " -- and not a bug in GameMode, it should be reported upstream.\n" - " -- If unsure, please also look here:\n" - " -- https://github.com/FeralInteractive/gamemode/issues/68\n"; - break; - } - case EINVAL: { - static int once = 0; - if (once++) - break; - additional_message = - " -- The error indicates that your kernel may not support this. If you\n" - " -- don't know what SCHED_ISO means, you can safely ignore this. If you\n" - " -- expected it to work, ensure you're running a kernel with MuQSS or\n" - " -- PDS scheduler.\n" - " -- For further technical reading on the topic start here:\n" - " -- https://lwn.net/Articles/720227/\n"; - break; - } - } - LOG_ERROR( - "Failed setting client [%d] into SCHED_ISO mode, ignoring error condition: %s\n%s", - client, - strerror(errno), - additional_message); - } - } else { - LOG_ERROR("Skipped setting client [%d] into SCHED_ISO mode: softrealtime setting is '%s'\n", - client, - softrealtime); - } -} - /** * Apply io priorities * @@ -539,7 +428,8 @@ bool game_mode_context_register(GameModeContext *self, pid_t client) } /* Apply scheduler policies */ - game_mode_apply_scheduler(self, client); + game_mode_apply_renice(self, client); + game_mode_apply_scheduling(self, client); /* Apply io priorities */ game_mode_apply_ioprio(self, client); diff --git a/daemon/gamemode.h b/daemon/gamemode.h index 201cf85..ab41576 100644 --- a/daemon/gamemode.h +++ b/daemon/gamemode.h @@ -110,3 +110,10 @@ char *game_mode_lookup_user_home(void); */ procfd_t game_mode_open_proc(const pid_t pid); int game_mode_close_proc(const procfd_t procfd); + +/** gamemode-sched.c + * Provides internal API functions specific to adjusting process + * scheduling. + */ +void game_mode_apply_renice(const GameModeContext *self, const pid_t client); +void game_mode_apply_scheduling(const GameModeContext *self, const pid_t client); diff --git a/daemon/meson.build b/daemon/meson.build index 5f4dcc2..ecc8181 100644 --- a/daemon/meson.build +++ b/daemon/meson.build @@ -20,6 +20,7 @@ daemon_sources = [ 'gamemode.c', 'gamemode-env.c', 'gamemode-proc.c', + 'gamemode-sched.c', 'daemonize.c', 'dbus_messaging.c', 'governors.c', From 403ce122f6fff7f619d2a81b9cb40d7a7715ab01 Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Mon, 8 Oct 2018 22:24:53 +0200 Subject: [PATCH 6/9] refactor: Break ioprio API functions out of the main daemon source Signed-off-by: Kai Krakow --- daemon/{ioprio.h => gamemode-ioprio.c} | 61 +++++++++++++++++++++++++- daemon/gamemode.c | 51 --------------------- daemon/gamemode.h | 6 +++ daemon/meson.build | 1 + 4 files changed, 66 insertions(+), 53 deletions(-) rename daemon/{ioprio.h => gamemode-ioprio.c} (59%) diff --git a/daemon/ioprio.h b/daemon/gamemode-ioprio.c similarity index 59% rename from daemon/ioprio.h rename to daemon/gamemode-ioprio.c index cddab7e..159a991 100644 --- a/daemon/ioprio.h +++ b/daemon/gamemode-ioprio.c @@ -29,9 +29,14 @@ POSSIBILITY OF SUCH DAMAGE. */ -#pragma once - #define _GNU_SOURCE + +#include "daemon_config.h" +#include "gamemode.h" +#include "helpers.h" +#include "logging.h" + +#include #include #include @@ -80,3 +85,55 @@ static inline int ioprio_set(int which, int who, int ioprio) { return (int)syscall(SYS_ioprio_set, which, who, ioprio); } + +/** + * Apply io priorities + * + * This tries to change the io priority of the client to a value specified + * 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) +{ + GameModeConfig *config = game_mode_config_from_context(self); + + LOG_MSG("Setting scheduling policies...\n"); + + /* + * read configuration "ioprio" (0..7) + */ + int ioprio = 0; + config_get_ioprio_value(config, &ioprio); + 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_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); + } + + /* + * 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", + client, + c, + p, + errno); + } +} diff --git a/daemon/gamemode.c b/daemon/gamemode.c index 79da96c..d046d1b 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -36,7 +36,6 @@ POSSIBILITY OF SUCH DAMAGE. #include "governors-query.h" #include "governors.h" #include "helpers.h" -#include "ioprio.h" #include "logging.h" #include @@ -159,56 +158,6 @@ void game_mode_context_destroy(GameModeContext *self) pthread_rwlock_destroy(&self->rwlock); } -/** - * Apply io priorities - * - * This tries to change the io priority of the client to a value specified - * and can possibly reduce lags or latency when a game has to load assets - * on demand. - */ -static void game_mode_apply_ioprio(GameModeContext *self, pid_t client) -{ - LOG_MSG("Setting scheduling policies...\n"); - - /* - * read configuration "ioprio" (0..7) - */ - int ioprio = 0; - config_get_ioprio_value(self->config, &ioprio); - 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_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); - } - - /* - * 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", - client, - c, - p, - errno); - } -} - /** * Pivot into game mode. * diff --git a/daemon/gamemode.h b/daemon/gamemode.h index ab41576..083b389 100644 --- a/daemon/gamemode.h +++ b/daemon/gamemode.h @@ -104,6 +104,12 @@ GameModeConfig *game_mode_config_from_context(const GameModeContext *context); char *game_mode_lookup_proc_env(const procfd_t proc_fd, const char *var); char *game_mode_lookup_user_home(void); +/** gamemode-ioprio.c + * Provides internal API functions specific to adjusting process + * IO priorities. + */ +void game_mode_apply_ioprio(const GameModeContext *self, const pid_t client); + /** gamemode-proc.c * Provides internal API functions specific to working with process * environments. diff --git a/daemon/meson.build b/daemon/meson.build index ecc8181..ebcf1d6 100644 --- a/daemon/meson.build +++ b/daemon/meson.build @@ -19,6 +19,7 @@ daemon_sources = [ 'main.c', 'gamemode.c', 'gamemode-env.c', + 'gamemode-ioprio.c', 'gamemode-proc.c', 'gamemode-sched.c', 'daemonize.c', From f4cd01f989435e34006628e30bbe557667485b52 Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Mon, 8 Oct 2018 22:40:35 +0200 Subject: [PATCH 7/9] refactor: Break wine API functions out of the main daemon source Signed-off-by: Kai Krakow --- daemon/gamemode-wine.c | 164 +++++++++++++++++++++++++++++++++++++++++ daemon/gamemode.c | 113 +--------------------------- daemon/gamemode.h | 8 ++ daemon/meson.build | 1 + 4 files changed, 175 insertions(+), 111 deletions(-) create mode 100644 daemon/gamemode-wine.c diff --git a/daemon/gamemode-wine.c b/daemon/gamemode-wine.c new file mode 100644 index 0000000..c2cfc66 --- /dev/null +++ b/daemon/gamemode-wine.c @@ -0,0 +1,164 @@ +/* + +Copyright (c) 2017-2018, Feral Interactive +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + * Neither the name of Feral Interactive nor the names of its contributors + may be used to endorse or promote products derived from this software + without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +POSSIBILITY OF SUCH DAMAGE. + + */ + +#define _GNU_SOURCE + +#include "gamemode.h" +#include "helpers.h" +#include "logging.h" + +#include +#include +#include +#include + +/** + * Detect if the process is a wine preloader process + */ +bool game_mode_detect_wine_preloader(const char *exe) +{ + return (strtail(exe, "/wine-preloader") || strtail(exe, "/wine64-preloader")); +} + +/** + * Detect if the process is a wine loader process + */ +bool game_mode_detect_wine_loader(const char *exe) +{ + return (strtail(exe, "/wine") || strtail(exe, "/wine64")); +} + +/** + * Attempt to resolve the exe for wine-preloader. + * This function is used if game_mode_context_find_exe() identified the + * process as wine-preloader. Returns NULL when resolve fails. + */ +char *game_mode_resolve_wine_preloader(const pid_t pid) +{ + char buffer[PATH_MAX]; + char *proc_path = NULL, *wine_exe = NULL, *wineprefix = NULL; + + /* Open the directory, we are potentially reading multiple files from it */ + procfd_t proc_fd = game_mode_open_proc(pid); + + if (proc_fd == INVALID_PROCFD) + goto fail_proc; + + /* Open the command line */ + int fd = openat(proc_fd, "cmdline", O_RDONLY | O_CLOEXEC); + if (fd != -1) { + FILE *stream = fdopen(fd, "r"); + if (stream) { + char *argv = NULL; + size_t args = 0; + int argc = 0; + while (!wine_exe && (argc++ < 2) && (getdelim(&argv, &args, 0, stream) != -1)) { + /* If we see the wine loader here, we have to use the next argument */ + if (strtail(argv, "/wine") || strtail(argv, "/wine64")) + continue; + free(wine_exe); // just in case + /* Check presence of the drive letter, we assume that below */ + wine_exe = args > 2 && argv[1] == ':' ? strndup(argv, args) : NULL; + } + free(argv); + fclose(stream); + } else + close(fd); + } + + /* Did we get wine exe from cmdline? */ + if (wine_exe) + LOG_MSG("Detected wine exe for client %d [%s].\n", pid, wine_exe); + else + goto fail_cmdline; + + /* Open the process environment and find the WINEPREFIX */ + errno = 0; + if (!(wineprefix = game_mode_lookup_proc_env(proc_fd, "WINEPREFIX"))) { + /* Lookup user home instead only if there was no error */ + char *home = NULL; + if (errno == 0) + home = game_mode_lookup_user_home(); + + /* Append "/.wine" if we found the user home */ + if (home) + wineprefix = safe_snprintf(buffer, "%s/.wine", home); + + /* Cleanup and check result */ + free(home); + if (!wineprefix) + goto fail_env; + } + + /* Wine prefix was detected, log this for diagnostics */ + LOG_MSG("Detected wine prefix for client %d: '%s'\n", pid, wineprefix); + + /* Convert Windows to Unix path separators */ + char *ix = wine_exe; + while (ix != NULL) + (ix = strchr(ix, '\\')) && (*ix++ = '/'); + + /* Convert the drive letter to lcase because wine handles it this way in the prefix */ + wine_exe[0] = (char)tolower(wine_exe[0]); + + /* Convert relative wine exe path to full unix path */ + char *wine_path = buffered_snprintf(buffer, "%s/dosdevices/%s", wineprefix, wine_exe); + free(wine_exe); + wine_exe = wine_path ? realpath(wine_path, NULL) : NULL; + + /* Fine? Successo? Fortuna! */ + if (wine_exe) + LOG_MSG("Successfully mapped wine client %d [%s].\n", pid, wine_exe); + else + goto fail; + +error_cleanup: + game_mode_close_proc(proc_fd); + free(wineprefix); + free(proc_path); + return wine_exe; + +fail: + LOG_ERROR("Unable to find wine executable for client %d: %s\n", pid, strerror(errno)); + goto error_cleanup; + +fail_cmdline: + LOG_ERROR("Wine loader has no accepted cmdline for client %d yet, deferring.\n", pid); + goto error_cleanup; + +fail_env: + LOG_ERROR("Failed to access process environment in '%s': %s\n", proc_path, strerror(errno)); + goto error_cleanup; + +fail_proc: + LOG_ERROR("Failed to access process data in '%s': %s\n", proc_path, strerror(errno)); + goto error_cleanup; +} diff --git a/daemon/gamemode.c b/daemon/gamemode.c index d046d1b..c1a2864 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -38,8 +38,6 @@ POSSIBILITY OF SUCH DAMAGE. #include "helpers.h" #include "logging.h" -#include -#include #include #include #include @@ -562,113 +560,6 @@ GameModeConfig *game_mode_config_from_context(const GameModeContext *context) return context ? context->config : NULL; } -/** - * Attempt to resolve the exe for wine-preloader. - * This function is used if game_mode_context_find_exe() identified the - * process as wine-preloader. Returns NULL when resolve fails. - */ -static char *game_mode_resolve_wine_preloader(pid_t pid) -{ - char buffer[PATH_MAX]; - char *proc_path = NULL, *wine_exe = NULL, *wineprefix = NULL; - - /* Open the directory, we are potentially reading multiple files from it */ - procfd_t proc_fd = game_mode_open_proc(pid); - - if (proc_fd == INVALID_PROCFD) - goto fail_proc; - - /* Open the command line */ - int fd = openat(proc_fd, "cmdline", O_RDONLY | O_CLOEXEC); - if (fd != -1) { - FILE *stream = fdopen(fd, "r"); - if (stream) { - char *argv = NULL; - size_t args = 0; - int argc = 0; - while (!wine_exe && (argc++ < 2) && (getdelim(&argv, &args, 0, stream) != -1)) { - /* If we see the wine loader here, we have to use the next argument */ - if (strtail(argv, "/wine") || strtail(argv, "/wine64")) - continue; - free(wine_exe); // just in case - /* Check presence of the drive letter, we assume that below */ - wine_exe = args > 2 && argv[1] == ':' ? strndup(argv, args) : NULL; - } - free(argv); - fclose(stream); - } else - close(fd); - } - - /* Did we get wine exe from cmdline? */ - if (wine_exe) - LOG_MSG("Detected wine exe for client %d [%s].\n", pid, wine_exe); - else - goto fail_cmdline; - - /* Open the process environment and find the WINEPREFIX */ - errno = 0; - if (!(wineprefix = game_mode_lookup_proc_env(proc_fd, "WINEPREFIX"))) { - /* Lookup user home instead only if there was no error */ - char *home = NULL; - if (errno == 0) - home = game_mode_lookup_user_home(); - - /* Append "/.wine" if we found the user home */ - if (home) - wineprefix = safe_snprintf(buffer, "%s/.wine", home); - - /* Cleanup and check result */ - free(home); - if (!wineprefix) - goto fail_env; - } - - /* Wine prefix was detected, log this for diagnostics */ - LOG_MSG("Detected wine prefix for client %d: '%s'\n", pid, wineprefix); - - /* Convert Windows to Unix path separators */ - char *ix = wine_exe; - while (ix != NULL) - (ix = strchr(ix, '\\')) && (*ix++ = '/'); - - /* Convert the drive letter to lcase because wine handles it this way in the prefix */ - wine_exe[0] = (char)tolower(wine_exe[0]); - - /* Convert relative wine exe path to full unix path */ - char *wine_path = buffered_snprintf(buffer, "%s/dosdevices/%s", wineprefix, wine_exe); - free(wine_exe); - wine_exe = wine_path ? realpath(wine_path, NULL) : NULL; - - /* Fine? Successo? Fortuna! */ - if (wine_exe) - LOG_MSG("Successfully mapped wine client %d [%s].\n", pid, wine_exe); - else - goto fail; - -error_cleanup: - game_mode_close_proc(proc_fd); - free(wineprefix); - free(proc_path); - return wine_exe; - -fail: - LOG_ERROR("Unable to find wine executable for client %d: %s\n", pid, strerror(errno)); - goto error_cleanup; - -fail_cmdline: - LOG_ERROR("Wine loader has no accepted cmdline for client %d yet, deferring.\n", pid); - goto error_cleanup; - -fail_env: - LOG_ERROR("Failed to access process environment in '%s': %s\n", proc_path, strerror(errno)); - goto error_cleanup; - -fail_proc: - LOG_ERROR("Failed to access process data in '%s': %s\n", proc_path, strerror(errno)); - goto error_cleanup; -} - /** * Attempt to locate the exe for the process. * We might run into issues if the process is running under an odd umask. @@ -687,11 +578,11 @@ static char *game_mode_context_find_exe(pid_t pid) goto fail; /* Detect if the process is a wine loader process */ - if (strtail(exe, "/wine-preloader") || strtail(exe, "/wine64-preloader")) { + if (game_mode_detect_wine_preloader(exe)) { LOG_MSG("Detected wine preloader for client %d [%s].\n", pid, exe); goto wine_preloader; } - if (strtail(exe, "/wine") || strtail(exe, "/wine64")) { + if (game_mode_detect_wine_loader(exe)) { LOG_MSG("Detected wine loader for client %d [%s].\n", pid, exe); goto wine_preloader; } diff --git a/daemon/gamemode.h b/daemon/gamemode.h index 083b389..f7f5c96 100644 --- a/daemon/gamemode.h +++ b/daemon/gamemode.h @@ -123,3 +123,11 @@ int game_mode_close_proc(const procfd_t procfd); */ void game_mode_apply_renice(const GameModeContext *self, const pid_t client); void game_mode_apply_scheduling(const GameModeContext *self, const pid_t client); + +/** gamemode-wine.c + * Provides internal API functions specific to handling wine + * prefixes. + */ +bool game_mode_detect_wine_loader(const char *exe); +bool game_mode_detect_wine_preloader(const char *exe); +char *game_mode_resolve_wine_preloader(const pid_t pid); diff --git a/daemon/meson.build b/daemon/meson.build index ebcf1d6..1e9fec8 100644 --- a/daemon/meson.build +++ b/daemon/meson.build @@ -22,6 +22,7 @@ daemon_sources = [ 'gamemode-ioprio.c', 'gamemode-proc.c', 'gamemode-sched.c', + 'gamemode-wine.c', 'daemonize.c', 'dbus_messaging.c', 'governors.c', From 5396370e5df49da2fc5162550e92a10dd7ab28b4 Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Tue, 9 Oct 2018 00:05:47 +0200 Subject: [PATCH 8/9] refactor: Simplify the log hinter Signed-off-by: Kai Krakow --- daemon/gamemode-ioprio.c | 5 ++- daemon/gamemode-sched.c | 72 ++++++++++++++++++---------------------- daemon/gamemode.c | 33 ++++++++---------- daemon/logging.h | 27 +++++++++++++++ 4 files changed, 77 insertions(+), 60 deletions(-) diff --git a/daemon/gamemode-ioprio.c b/daemon/gamemode-ioprio.c index 159a991..44a459b 100644 --- a/daemon/gamemode-ioprio.c +++ b/daemon/gamemode-ioprio.c @@ -114,7 +114,10 @@ void game_mode_apply_ioprio(const GameModeContext *self, const pid_t client) int invalid_ioprio = ioprio; ioprio = CLAMP(0, 7, ioprio); if (ioprio != invalid_ioprio) - LOG_ERROR("IO priority value %d invalid, clamping to %d\n", invalid_ioprio, 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); diff --git a/daemon/gamemode-sched.c b/daemon/gamemode-sched.c index cd2a2ed..e0f244c 100644 --- a/daemon/gamemode-sched.c +++ b/daemon/gamemode-sched.c @@ -67,9 +67,10 @@ void game_mode_apply_renice(const GameModeContext *self, const pid_t client) long int renice = 0; config_get_renice_value(config, &renice); if ((renice < 1) || (renice > 20)) { - LOG_ERROR("Invalid renice value '%ld' reset to default: %d.\n", - renice, - -NICE_DEFAULT_PRIORITY); + LOG_ONCE(ERROR, + "Invalid renice value '%ld' reset to default: %d.\n", + renice, + -NICE_DEFAULT_PRIORITY); renice = NICE_DEFAULT_PRIORITY; } else { renice = -renice; @@ -81,12 +82,12 @@ void game_mode_apply_renice(const GameModeContext *self, const pid_t client) if (getpriority(PRIO_PROCESS, (id_t)client) != 0) { LOG_ERROR("Refused to renice client [%d]: already reniced\n", client); } else if (setpriority(PRIO_PROCESS, (id_t)client, (int)renice)) { - LOG_ERROR( - "Failed to renice client [%d], ignoring error condition: %s\n" - " -- Your user may not have permission to do this. Please read the docs\n" - " -- to learn how to adjust the pam limits.\n", - client, - strerror(errno)); + LOG_HINTED(ERROR, + "Failed to renice client [%d], ignoring error condition: %s\n", + " -- Your user may not have permission to do this. Please read the docs\n" + " -- to learn how to adjust the pam limits.\n", + client, + strerror(errno)); } } @@ -114,40 +115,31 @@ void game_mode_apply_scheduling(const GameModeContext *self, const pid_t client) if (!(strcmp(softrealtime, "off") == 0) && (enable_softrealtime)) { const struct sched_param p = { .sched_priority = 0 }; if (sched_setscheduler(client, SCHED_ISO | SCHED_RESET_ON_FORK, &p)) { - const char *additional_message = ""; - switch (errno) { - case EPERM: { - static int once = 0; - if (once++) - break; - additional_message = - " -- The error indicates that you may be running a resource management\n" - " -- daemon managing your game launcher and it leaks lower scheduling\n" - " -- classes into the games. This is likely a bug in the management daemon\n" - " -- and not a bug in GameMode, it should be reported upstream.\n" - " -- If unsure, please also look here:\n" - " -- https://github.com/FeralInteractive/gamemode/issues/68\n"; - break; - } - case EINVAL: { - static int once = 0; - if (once++) - break; - additional_message = - " -- The error indicates that your kernel may not support this. If you\n" - " -- don't know what SCHED_ISO means, you can safely ignore this. If you\n" - " -- expected it to work, ensure you're running a kernel with MuQSS or\n" - " -- PDS scheduler.\n" - " -- For further technical reading on the topic start here:\n" - " -- https://lwn.net/Articles/720227/\n"; - break; - } - } + const char *hint = ""; + HINT_ONCE_ON( + errno == EPERM, + hint, + " -- The error indicates that you may be running a resource management\n" + " -- daemon managing your game launcher and it leaks lower scheduling\n" + " -- classes into the games. This is likely a bug in the management daemon\n" + " -- and not a bug in GameMode, it should be reported upstream.\n" + " -- If unsure, please also look here:\n" + " -- https://github.com/FeralInteractive/gamemode/issues/68\n"); + HINT_ONCE_ON( + errno == EINVAL, + hint, + " -- The error indicates that your kernel may not support this. If you\n" + " -- don't know what SCHED_ISO means, you can safely ignore this. If you\n" + " -- expected it to work, ensure you're running a kernel with MuQSS or\n" + " -- PDS scheduler.\n" + " -- For further technical reading on the topic start here:\n" + " -- https://lwn.net/Articles/720227/\n"); LOG_ERROR( - "Failed setting client [%d] into SCHED_ISO mode, ignoring error condition: %s\n%s", + "Failed setting client [%d] into SCHED_ISO mode, ignoring error condition: %s\n" + "%s", client, strerror(errno), - additional_message); + hint); } } else { LOG_ERROR("Skipped setting client [%d] into SCHED_ISO mode: softrealtime setting is '%s'\n", diff --git a/daemon/gamemode.c b/daemon/gamemode.c index c1a2864..c844897 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -323,16 +323,12 @@ bool game_mode_context_register(GameModeContext *self, pid_t client) pthread_rwlock_rdlock(&self->rwlock); // ensure our pointer is sane const GameModeClient *existing = game_mode_context_has_client(self, client); if (existing) { - static int once = 0; - const char *additional_message = - (once++ - ? "" - : " -- This may happen due to using exec or shell wrappers. You may want to\n" - " -- blacklist this client so GameMode can see its final name here.\n"); - LOG_ERROR("Addition requested for already known client %d [%s].\n%s", - existing->pid, - existing->executable, - additional_message); + LOG_HINTED(ERROR, + "Addition requested for already known client %d [%s].\n", + " -- This may happen due to using exec or shell wrappers. You may want to\n" + " -- blacklist this client so GameMode can see its final name here.\n", + existing->pid, + existing->executable); pthread_rwlock_unlock(&self->rwlock); goto error_cleanup; } @@ -423,15 +419,14 @@ bool game_mode_context_unregister(GameModeContext *self, pid_t client) pthread_rwlock_unlock(&self->rwlock); if (!found) { - static int once = 0; - const char *additional_message = - (once++ - ? "" - : " -- The parent process probably forked and tries to unregister from the\n" - " -- wrong process now. We cannot work around this. This message will\n" - " -- likely be paired with a nearby 'Removing expired game' which means we\n" - " -- cleaned up properly (we will log this event).\n"); - LOG_ERROR("Removal requested for unknown process [%d].\n%s", client, additional_message); + LOG_HINTED( + ERROR, + "Removal requested for unknown process [%d].\n", + " -- The parent process probably forked and tries to unregister from the wrong\n" + " -- process now. We cannot work around this. This message will likely be paired\n" + " -- with a nearby 'Removing expired game' which means we cleaned up properly\n" + " -- (we will log this event). This hint will be displayed only once.\n", + client); return false; } diff --git a/daemon/logging.h b/daemon/logging.h index 51d7690..a45996a 100644 --- a/daemon/logging.h +++ b/daemon/logging.h @@ -62,6 +62,13 @@ POSSIBILITY OF SUCH DAMAGE. } \ } while (0) +#define LOG_ONCE(type, ...) \ + do { \ + static int __once = 0; \ + if (!__once++) \ + LOG_##type(__VA_ARGS__); \ + } while (0) + /* Fatal warnings trigger an exit */ #define FATAL_ERRORNO(msg) \ do { \ @@ -74,6 +81,26 @@ POSSIBILITY OF SUCH DAMAGE. exit(EXIT_FAILURE); \ } while (0) +/* Hinting helpers */ +#define HINT_ONCE(name, hint) \ + do { \ + static int __once = 0; \ + name = (!__once++ ? hint : ""); \ + } while (0) + +#define HINT_ONCE_ON(cond, ...) \ + do { \ + if (cond) \ + HINT_ONCE(__VA_ARGS__); \ + } while (0); + +#define LOG_HINTED(type, msg, hint, ...) \ + do { \ + const char *__arg; \ + HINT_ONCE(__arg, hint); \ + LOG_##type(msg "%s", __VA_ARGS__, __arg); \ + } while (0) + /** * Control if and how how we use syslog */ From b2870671bd38379c7d93ad396abc0bf1d5d96006 Mon Sep 17 00:00:00 2001 From: Kai Krakow Date: Tue, 9 Oct 2018 00:39:12 +0200 Subject: [PATCH 9/9] formatcheck: Use a shorter timeout If the internet connection is down, the default timeout of wget is unreasonably long. If formatcheck is used as a pre-commit hook, this blocks usage of git for a long time although we probably have git-clang-format available. Signed-off-by: Kai Krakow --- scripts/format-check.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/format-check.sh b/scripts/format-check.sh index a2384e0..00306c2 100755 --- a/scripts/format-check.sh +++ b/scripts/format-check.sh @@ -4,7 +4,7 @@ # Ensure we are at the project root cd "$(dirname $0)"/.. -wget -Nq https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format +wget -Nq -T3 -t1 https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format if chmod +x git-clang-format; then if [[ "$1" == "--pre-commit" ]]; then