From ceb1808c9501d04434324565b1b7229034940277 Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Tue, 14 May 2019 21:16:24 +0100 Subject: [PATCH 1/7] Initial implementation of a RefreshConfig dbus interface --- daemon/dbus_messaging.c | 12 ++++ daemon/gamemode.c | 129 ++++++++++++++++++++++++++++++---------- daemon/gamemode.h | 5 ++ 3 files changed, 116 insertions(+), 30 deletions(-) diff --git a/daemon/dbus_messaging.c b/daemon/dbus_messaging.c index 0a9fe20..4c03b14 100644 --- a/daemon/dbus_messaging.c +++ b/daemon/dbus_messaging.c @@ -206,6 +206,17 @@ void game_mode_client_count_changed(void) NULL); } +/** + * Handles the Refresh Config request + */ +static int method_refresh_config(sd_bus_message *m, void *userdata, + __attribute__((unused)) sd_bus_error *ret_error) +{ + GameModeContext *context = userdata; + int status = game_mode_refresh_config(context); + return sd_bus_reply_method_return(m, "i", status); +} + /** * D-BUS vtable to dispatch virtual methods */ @@ -222,6 +233,7 @@ static const sd_bus_vtable gamemode_vtable[] = { SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("QueryStatusByPID", "ii", "i", method_query_status_by_pid, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("RefreshConfig", "", "i", method_refresh_config, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_VTABLE_END }; diff --git a/daemon/gamemode.c b/daemon/gamemode.c index 6b977f8..4532db2 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -99,6 +99,17 @@ 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); +static void start_reaper_thread(GameModeContext *self) +{ + pthread_mutex_init(&self->reaper.mutex, NULL); + pthread_cond_init(&self->reaper.condition, NULL); + + self->reaper.running = true; + if (pthread_create(&self->reaper.thread, NULL, game_mode_context_reaper, self) != 0) { + FATAL_ERROR("Couldn't construct a new thread"); + } +} + void game_mode_context_init(GameModeContext *self) { if (had_context_init) { @@ -120,14 +131,25 @@ void game_mode_context_init(GameModeContext *self) game_mode_initialise_gpu(self->config, &self->target_gpu); pthread_rwlock_init(&self->rwlock, NULL); - pthread_mutex_init(&self->reaper.mutex, NULL); - pthread_cond_init(&self->reaper.condition, NULL); /* Get the reaper thread going */ - self->reaper.running = true; - if (pthread_create(&self->reaper.thread, NULL, game_mode_context_reaper, self) != 0) { - FATAL_ERROR("Couldn't construct a new thread"); - } + start_reaper_thread(self); +} + +static void end_reaper_thread(GameModeContext *self) +{ + self->reaper.running = false; + + /* We might be stuck waiting, so wake it up again */ + pthread_mutex_lock(&self->reaper.mutex); + pthread_cond_signal(&self->reaper.condition); + pthread_mutex_unlock(&self->reaper.mutex); + + /* Join the thread as soon as possible */ + pthread_join(self->reaper.thread, NULL); + + pthread_cond_destroy(&self->reaper.condition); + pthread_mutex_destroy(&self->reaper.mutex); } void game_mode_context_destroy(GameModeContext *self) @@ -143,18 +165,8 @@ void game_mode_context_destroy(GameModeContext *self) had_context_init = false; game_mode_client_free(self->client); - self->reaper.running = false; - /* We might be stuck waiting, so wake it up again */ - pthread_mutex_lock(&self->reaper.mutex); - pthread_cond_signal(&self->reaper.condition); - pthread_mutex_unlock(&self->reaper.mutex); - - /* Join the thread as soon as possible */ - pthread_join(self->reaper.thread, NULL); - - pthread_cond_destroy(&self->reaper.condition); - pthread_mutex_destroy(&self->reaper.mutex); + end_reaper_thread(self); /* Destroy the gpu object */ game_mode_free_gpu(&self->stored_gpu); @@ -324,6 +336,20 @@ int game_mode_context_num_clients(GameModeContext *self) return atomic_load(&self->refcount); } +static int game_mode_apply_client_optimisations(GameModeContext *self, pid_t client) +{ + /* Store current renice and apply */ + game_mode_apply_renice(self, client, 0 /* expect zero value to start with */); + + /* Store current ioprio value and apply */ + game_mode_apply_ioprio(self, client, IOPRIO_DEFAULT); + + /* Apply scheduler policies */ + game_mode_apply_scheduling(self, client); + + return 0; +} + int game_mode_context_register(GameModeContext *self, pid_t client, pid_t requester) { errno = 0; @@ -412,14 +438,7 @@ int game_mode_context_register(GameModeContext *self, pid_t client, pid_t reques game_mode_context_enter(self); } - /* Store current renice and apply */ - game_mode_apply_renice(self, client, 0 /* expect zero value to start with */); - - /* Store current ioprio value and apply */ - game_mode_apply_ioprio(self, client, IOPRIO_DEFAULT); - - /* Apply scheduler policies */ - game_mode_apply_scheduling(self, client); + game_mode_apply_client_optimisations(self, client); game_mode_client_count_changed(); @@ -433,6 +452,17 @@ error_cleanup: return -1; } +static int game_mode_remove_client_optimisations(GameModeContext *self, pid_t client) +{ + /* Restore the ioprio value for the process, expecting it to be the config value */ + game_mode_apply_ioprio(self, client, (int)config_get_ioprio_value(self->config)); + + /* Restore the renice value for the process, expecting it to be our config value */ + game_mode_apply_renice(self, client, (int)config_get_renice_value(self->config)); + + return 0; +} + int game_mode_context_unregister(GameModeContext *self, pid_t client, pid_t requester) { GameModeClient *cl = NULL; @@ -508,11 +538,7 @@ int game_mode_context_unregister(GameModeContext *self, pid_t client, pid_t requ game_mode_client_count_changed(); - /* Restore the ioprio value for the process, expecting it to be the config value */ - game_mode_apply_ioprio(self, client, (int)config_get_ioprio_value(self->config)); - - /* Restore the renice value for the process, expecting it to be our config value */ - game_mode_apply_renice(self, client, (int)config_get_renice_value(self->config)); + game_mode_remove_client_optimisations(self, client); return 0; } @@ -734,3 +760,46 @@ static void game_mode_execute_scripts(char scripts[CONFIG_LIST_MAX][CONFIG_VALUE i++; } } + +/* + * Refresh the current configuration + * + * Refreshing the configuration live would be problematic for various optimisation values, to ensure + * we have a fully clean state, we tear down the whole gamemode state and regrow it with a new + * config, remembering the registered games + */ +int game_mode_refresh_config(GameModeContext *self) +{ + LOG_MSG("Refreshing config..."); + + /* Stop the reaper thread first */ + end_reaper_thread(self); + + /* Remove current client optimisations */ + pthread_rwlock_rdlock(&self->rwlock); + for (GameModeClient *cl = self->client; cl; cl = cl->next) + game_mode_remove_client_optimisations(self, cl->pid); + pthread_rwlock_unlock(&self->rwlock); + + /* Shut down the global context */ + game_mode_context_leave(self); + + /* Reload the config */ + config_reload(self->config); + + /* Start the global context back up */ + game_mode_context_enter(self); + + /* Re-apply all current optimisations */ + pthread_rwlock_rdlock(&self->rwlock); + for (GameModeClient *cl = self->client; cl; cl = cl->next) + game_mode_apply_client_optimisations(self, cl->pid); + pthread_rwlock_unlock(&self->rwlock); + + /* Restart the reaper thread back up again */ + start_reaper_thread(self); + + LOG_MSG("Config refreshed..."); + + return 0; +} \ No newline at end of file diff --git a/daemon/gamemode.h b/daemon/gamemode.h index d5d6eb6..b9ddad1 100644 --- a/daemon/gamemode.h +++ b/daemon/gamemode.h @@ -111,6 +111,11 @@ int game_mode_context_query_status(GameModeContext *self, pid_t pid, pid_t reque */ GameModeConfig *game_mode_config_from_context(const GameModeContext *context); +/* + * Refresh the current configuration + */ +int game_mode_refresh_config(GameModeContext *context); + /** gamemode-env.c * Provides internal API functions specific to working environment * variables. From ec55bda3b295e0a0b62a9268d6e90166081bd3b1 Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Wed, 15 May 2019 18:00:43 +0100 Subject: [PATCH 2/7] Implement inotify check on the reaper thread for config changes --- daemon/daemon_config.c | 127 +++++++++++++++++++++++++++++++++++++--- daemon/daemon_config.h | 5 ++ daemon/dbus_messaging.c | 2 +- daemon/gamemode.c | 70 +++++++++++++--------- daemon/gamemode.h | 4 +- 5 files changed, 169 insertions(+), 39 deletions(-) diff --git a/daemon/daemon_config.c b/daemon/daemon_config.c index a03147b..550e9ef 100644 --- a/daemon/daemon_config.c +++ b/daemon/daemon_config.c @@ -37,11 +37,14 @@ POSSIBILITY OF SUCH DAMAGE. /* Ben Hoyt's inih library */ #include "ini.h" +#include #include #include #include #include #include +#include +#include #include /* Name and possible location of the config file */ @@ -59,6 +62,9 @@ POSSIBILITY OF SUCH DAMAGE. return value; \ } +/* The number of current locations for config files */ +#define CONFIG_NUM_LOCATIONS 4 + /** * The config holds various details as needed * and a rwlock to allow config_reload to be called @@ -66,7 +72,7 @@ POSSIBILITY OF SUCH DAMAGE. struct GameModeConfig { pthread_rwlock_t rwlock; int inotfd; - int inotwd; + int inotwd[CONFIG_NUM_LOCATIONS]; struct { char whitelist[CONFIG_LIST_MAX][CONFIG_VALUE_MAX]; @@ -342,7 +348,7 @@ static void load_config_files(GameModeConfig *self) const char *path; bool protected; }; - struct ConfigLocation locations[] = { + struct ConfigLocation locations[CONFIG_NUM_LOCATIONS] = { { "/usr/share/gamemode", true }, /* shipped default config */ { "/etc", true }, /* administrator config */ { config_location_home, false }, /* $XDG_CONFIG_HOME or $HOME/.config/ */ @@ -350,11 +356,12 @@ static void load_config_files(GameModeConfig *self) }; /* Load each file in order and overwrite values */ - for (unsigned int i = 0; i < sizeof(locations) / sizeof(locations[0]); i++) { + for (unsigned int i = 0; i < CONFIG_NUM_LOCATIONS; i++) { char *path = NULL; if (locations[i].path && asprintf(&path, "%s/" CONFIG_NAME, locations[i].path) > 0) { - FILE *f = fopen(path, "r"); - if (f) { + FILE *f = NULL; + DIR *d = NULL; + if ((f = fopen(path, "r"))) { LOG_MSG("Loading config file [%s]\n", path); load_protected = locations[i].protected; int error = ini_parse_file(f, inih_handler, (void *)self); @@ -364,6 +371,24 @@ static void load_config_files(GameModeConfig *self) LOG_MSG("Failed to parse config file - error on line %d!\n", error); } fclose(f); + + /* Register for inotify */ + /* Watch for modification, deletion, moves, or attribute changes */ + uint32_t fileflags = IN_MODIFY | IN_DELETE_SELF | IN_MOVE_SELF | IN_ATTRIB; + if ((self->inotwd[i] = inotify_add_watch(self->inotfd, path, fileflags)) == -1) { + LOG_ERROR("Failed to watch %s, error: %s", path, strerror(errno)); + } + + } else if ((d = opendir(locations[i].path))) { + /* We didn't find a file, so we'll wait on the directory */ + /* Notify if a file is created, or move to the directory, or if the directory itself + * is removed or moved away */ + uint32_t dirflags = IN_CREATE | IN_MOVED_TO | IN_DELETE_SELF | IN_MOVE_SELF; + if ((self->inotwd[i] = + inotify_add_watch(self->inotfd, locations[i].path, dirflags)) == -1) { + LOG_ERROR("Failed to watch %s, error: %s", path, strerror(errno)); + } + closedir(d); } free(path); } @@ -409,16 +434,104 @@ void config_init(GameModeConfig *self) { pthread_rwlock_init(&self->rwlock, NULL); + self->inotfd = inotify_init1(IN_NONBLOCK); + if (self->inotfd == -1) + LOG_ERROR( + "inotify_init failed: %s, gamemode will be able to watch config files for edits!\n", + strerror(errno)); + + for (unsigned int i = 0; i < CONFIG_NUM_LOCATIONS; i++) { + self->inotwd[i] = -1; + } + /* load the initial config */ load_config_files(self); } +/* + * Destroy internal parts of config + */ +static void internal_destroy(GameModeConfig *self) +{ + pthread_rwlock_destroy(&self->rwlock); + + for (unsigned int i = 0; i < CONFIG_NUM_LOCATIONS; i++) { + if (self->inotwd[i] != -1) { + /* TODO: Error handle */ + inotify_rm_watch(self->inotfd, self->inotwd[i]); + } + } + + if (self->inotfd != -1) + close(self->inotfd); +} + /* * Re-load the config file */ void config_reload(GameModeConfig *self) { - load_config_files(self); + internal_destroy(self); + + config_init(self); +} + +/* + * Check if the config needs to be reloaded + */ +bool config_needs_reload(GameModeConfig *self) +{ + bool need = false; + + /* Take a read lock while we use the inotify fd */ + pthread_rwlock_rdlock(&self->rwlock); + + const size_t buflen = sizeof(struct inotify_event) + NAME_MAX + 1; + char buffer[buflen] __attribute__((aligned(__alignof__(struct inotify_event)))); + + ssize_t len = read(self->inotfd, buffer, buflen); + if (len == -1) { + /* EAGAIN is returned when there's nothing to read on a non-blocking fd */ + if (errno != EAGAIN) + LOG_ERROR("Could not read inotify fd: %s\n", strerror(errno)); + } else if (len > 0) { + /* Iterate over each event we've been given */ + size_t i = 0; + while (i < (size_t)len) { + struct inotify_event *event = (struct inotify_event *)&buffer[i]; + /* We have picked up an event and need to handle it */ + if (event->mask & IN_ISDIR) { + /* If the event is a dir event we need to take a look */ + if (event->mask & IN_DELETE_SELF || event->mask & IN_MOVE_SELF) { + /* The directory itself changed, trigger a reload */ + need = true; + break; + + } else if (event->mask & IN_CREATE || event->mask & IN_MOVED_TO) { + /* A new file has appeared, check the file name */ + if (strncmp(basename(event->name), CONFIG_NAME, strlen(CONFIG_NAME)) == 0) { + /* This is a gamemode config file, trigger a reload */ + need = true; + break; + } + } + + } else { + /* When the event isn't a dir event - one of our files has been interacted with + * in some way, so let's reload regardless of the details + */ + need = true; + break; + } + + i += sizeof(struct inotify_event) + event->len; + } + } + + /* Return the read lock */ + pthread_rwlock_unlock(&self->rwlock); + + return need; } /* @@ -426,7 +539,7 @@ void config_reload(GameModeConfig *self) */ void config_destroy(GameModeConfig *self) { - pthread_rwlock_destroy(&self->rwlock); + internal_destroy(self); /* Finally, free the memory */ free(self); diff --git a/daemon/daemon_config.h b/daemon/daemon_config.h index 790d26c..6550666 100644 --- a/daemon/daemon_config.h +++ b/daemon/daemon_config.h @@ -68,6 +68,11 @@ void config_init(GameModeConfig *self); */ void config_reload(GameModeConfig *self); +/* + * Check if the config has changed and will need a reload + */ +bool config_needs_reload(GameModeConfig *self); + /* * Destroy a config * Invalidates the config diff --git a/daemon/dbus_messaging.c b/daemon/dbus_messaging.c index 4c03b14..ca729e4 100644 --- a/daemon/dbus_messaging.c +++ b/daemon/dbus_messaging.c @@ -213,7 +213,7 @@ static int method_refresh_config(sd_bus_message *m, void *userdata, __attribute__((unused)) sd_bus_error *ret_error) { GameModeContext *context = userdata; - int status = game_mode_refresh_config(context); + int status = game_mode_reload_config(context); return sd_bus_reply_method_return(m, "i", status); } diff --git a/daemon/gamemode.c b/daemon/gamemode.c index 4532db2..edf21d6 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -637,6 +637,35 @@ static void game_mode_client_free(GameModeClient *client) free(client); } +/* Internal refresh config function (assumes no contention with reaper thread) */ +static void game_mode_reload_config_internal(GameModeContext *self) +{ + LOG_MSG("Reloading config...\n"); + + /* Remove current client optimisations */ + pthread_rwlock_rdlock(&self->rwlock); + for (GameModeClient *cl = self->client; cl; cl = cl->next) + game_mode_remove_client_optimisations(self, cl->pid); + pthread_rwlock_unlock(&self->rwlock); + + /* Shut down the global context */ + game_mode_context_leave(self); + + /* Reload the config */ + config_reload(self->config); + + /* Start the global context back up */ + game_mode_context_enter(self); + + /* Re-apply all current optimisations */ + pthread_rwlock_rdlock(&self->rwlock); + for (GameModeClient *cl = self->client; cl; cl = cl->next) + game_mode_apply_client_optimisations(self, cl->pid); + pthread_rwlock_unlock(&self->rwlock); + + LOG_MSG("Config reload complete\n"); +} + /** * We continuously run until told otherwise. */ @@ -664,6 +693,12 @@ static void *game_mode_context_reaper(void *userdata) /* Expire remaining entries */ game_mode_context_auto_expire(self); + /* Check if we should be reloading the config, and do so if needed */ + if (config_needs_reload(self->config)) { + LOG_MSG("Detected config file changes\n"); + game_mode_reload_config_internal(self); + } + ts.tv_sec = time(NULL) + reaper_interval; } @@ -762,44 +797,21 @@ static void game_mode_execute_scripts(char scripts[CONFIG_LIST_MAX][CONFIG_VALUE } /* - * Refresh the current configuration + * Reload the current configuration * - * Refreshing the configuration live would be problematic for various optimisation values, to ensure - * we have a fully clean state, we tear down the whole gamemode state and regrow it with a new - * config, remembering the registered games + * Reloading the configuration completely live would be problematic for various optimisation values, + * to ensure we have a fully clean state, we tear down the whole gamemode state and regrow it with a + * new config, remembering the registered games */ -int game_mode_refresh_config(GameModeContext *self) +int game_mode_reload_config(GameModeContext *self) { - LOG_MSG("Refreshing config..."); - /* Stop the reaper thread first */ end_reaper_thread(self); - /* Remove current client optimisations */ - pthread_rwlock_rdlock(&self->rwlock); - for (GameModeClient *cl = self->client; cl; cl = cl->next) - game_mode_remove_client_optimisations(self, cl->pid); - pthread_rwlock_unlock(&self->rwlock); - - /* Shut down the global context */ - game_mode_context_leave(self); - - /* Reload the config */ - config_reload(self->config); - - /* Start the global context back up */ - game_mode_context_enter(self); - - /* Re-apply all current optimisations */ - pthread_rwlock_rdlock(&self->rwlock); - for (GameModeClient *cl = self->client; cl; cl = cl->next) - game_mode_apply_client_optimisations(self, cl->pid); - pthread_rwlock_unlock(&self->rwlock); + game_mode_reload_config_internal(self); /* Restart the reaper thread back up again */ start_reaper_thread(self); - LOG_MSG("Config refreshed..."); - return 0; } \ No newline at end of file diff --git a/daemon/gamemode.h b/daemon/gamemode.h index b9ddad1..1545aeb 100644 --- a/daemon/gamemode.h +++ b/daemon/gamemode.h @@ -112,9 +112,9 @@ int game_mode_context_query_status(GameModeContext *self, pid_t pid, pid_t reque GameModeConfig *game_mode_config_from_context(const GameModeContext *context); /* - * Refresh the current configuration + * Reload the current configuration */ -int game_mode_refresh_config(GameModeContext *context); +int game_mode_reload_config(GameModeContext *context); /** gamemode-env.c * Provides internal API functions specific to working environment From 5d0a4130356ea8f4e0b42ccb5d05eade8245eebb Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Wed, 15 May 2019 18:38:36 +0100 Subject: [PATCH 3/7] Only perform the leave/enter loop when gamemode is already active/has clients --- daemon/gamemode.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/daemon/gamemode.c b/daemon/gamemode.c index edf21d6..aaac396 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -642,25 +642,29 @@ static void game_mode_reload_config_internal(GameModeContext *self) { LOG_MSG("Reloading config...\n"); - /* Remove current client optimisations */ - pthread_rwlock_rdlock(&self->rwlock); - for (GameModeClient *cl = self->client; cl; cl = cl->next) - game_mode_remove_client_optimisations(self, cl->pid); - pthread_rwlock_unlock(&self->rwlock); + /* Make sure we have a readwrite lock on ourselves */ + pthread_rwlock_wrlock(&self->rwlock); - /* Shut down the global context */ - game_mode_context_leave(self); + /* Remove current optimisations when we're already active */ + if (game_mode_context_num_clients(self)) { + for (GameModeClient *cl = self->client; cl; cl = cl->next) + game_mode_remove_client_optimisations(self, cl->pid); + + game_mode_context_leave(self); + } /* Reload the config */ config_reload(self->config); - /* Start the global context back up */ - game_mode_context_enter(self); - /* Re-apply all current optimisations */ - pthread_rwlock_rdlock(&self->rwlock); - for (GameModeClient *cl = self->client; cl; cl = cl->next) - game_mode_apply_client_optimisations(self, cl->pid); + if (game_mode_context_num_clients(self)) { + /* Start the global context back up */ + game_mode_context_enter(self); + + for (GameModeClient *cl = self->client; cl; cl = cl->next) + game_mode_apply_client_optimisations(self, cl->pid); + } + pthread_rwlock_unlock(&self->rwlock); LOG_MSG("Config reload complete\n"); From c276f760c7fe598d943a16b4c55cb4f733cbfaee Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Wed, 15 May 2019 18:42:00 +0100 Subject: [PATCH 4/7] Add comment about the reaper thread dealing with config file changes --- example/gamemode.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/gamemode.ini b/example/gamemode.ini index 86c0f78..d9e08d0 100644 --- a/example/gamemode.ini +++ b/example/gamemode.ini @@ -1,5 +1,5 @@ [general] -; The reaper thread will check every 5 seconds for exited clients +; The reaper thread will check every 5 seconds for exited clients and for config file changes reaper_freq=5 ; The desired governor is used when entering GameMode instead of "performance" From 89904602e9d8d56388ab10ddc940da3e665ede67 Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Sun, 19 May 2019 11:24:51 +0100 Subject: [PATCH 5/7] Fix formatting for travis --- daemon/dbus_messaging.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/daemon/dbus_messaging.c b/daemon/dbus_messaging.c index ca729e4..20b734e 100644 --- a/daemon/dbus_messaging.c +++ b/daemon/dbus_messaging.c @@ -220,22 +220,21 @@ static int method_refresh_config(sd_bus_message *m, void *userdata, /** * D-BUS vtable to dispatch virtual methods */ -static const sd_bus_vtable gamemode_vtable[] = { - SD_BUS_VTABLE_START(0), - SD_BUS_PROPERTY("ClientCount", "i", property_get_client_count, 0, - SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), - SD_BUS_METHOD("RegisterGame", "i", "i", method_register_game, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("UnregisterGame", "i", "i", method_unregister_game, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("QueryStatus", "i", "i", method_query_status, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("RegisterGameByPID", "ii", "i", method_register_game_by_pid, - SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("UnregisterGameByPID", "ii", "i", method_unregister_game_by_pid, - SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("QueryStatusByPID", "ii", "i", method_query_status_by_pid, - SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("RefreshConfig", "", "i", method_refresh_config, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_VTABLE_END -}; +static const sd_bus_vtable gamemode_vtable[] = + { SD_BUS_VTABLE_START(0), + SD_BUS_PROPERTY("ClientCount", "i", property_get_client_count, 0, + SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_METHOD("RegisterGame", "i", "i", method_register_game, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("UnregisterGame", "i", "i", method_unregister_game, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("QueryStatus", "i", "i", method_query_status, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("RegisterGameByPID", "ii", "i", method_register_game_by_pid, + SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("UnregisterGameByPID", "ii", "i", method_unregister_game_by_pid, + SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("QueryStatusByPID", "ii", "i", method_query_status_by_pid, + SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("RefreshConfig", "", "i", method_refresh_config, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_VTABLE_END }; /** * Main process loop for the daemon. Run until quitting has been requested. From a9e3f866a04ecd7898b026fc9f06850a44530050 Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Fri, 24 May 2019 08:45:14 +0100 Subject: [PATCH 6/7] Extend the context rwlock around where we apply optimisations This prevents a potential race condition for when the reaper thread reloads the config --- daemon/gamemode.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/daemon/gamemode.c b/daemon/gamemode.c index aaac396..444b123 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -431,7 +431,6 @@ int game_mode_context_register(GameModeContext *self, pid_t client, pid_t reques /* Update the list */ cl->next = self->client; self->client = cl; - pthread_rwlock_unlock(&self->rwlock); /* First add, init */ if (atomic_fetch_add_explicit(&self->refcount, 1, memory_order_seq_cst) == 0) { @@ -439,6 +438,7 @@ int game_mode_context_register(GameModeContext *self, pid_t client, pid_t reques } game_mode_apply_client_optimisations(self, client); + pthread_rwlock_unlock(&self->rwlock); game_mode_client_count_changed(); @@ -517,7 +517,6 @@ int game_mode_context_unregister(GameModeContext *self, pid_t client, pid_t requ } /* Unlock here, potentially yielding */ - pthread_rwlock_unlock(&self->rwlock); if (!found) { LOG_HINTED( @@ -528,6 +527,7 @@ int game_mode_context_unregister(GameModeContext *self, pid_t client, pid_t requ " -- 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); + pthread_rwlock_unlock(&self->rwlock); return -1; } @@ -536,10 +536,12 @@ int game_mode_context_unregister(GameModeContext *self, pid_t client, pid_t requ game_mode_context_leave(self); } - game_mode_client_count_changed(); - game_mode_remove_client_optimisations(self, client); + pthread_rwlock_unlock(&self->rwlock); + + game_mode_client_count_changed(); + return 0; } From a0b850474ddc627747617f86d51a8e1c2ec5590b Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Fri, 24 May 2019 08:48:38 +0100 Subject: [PATCH 7/7] Fix comments --- daemon/gamemode.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/daemon/gamemode.c b/daemon/gamemode.c index 444b123..cb8acd7 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -438,6 +438,8 @@ int game_mode_context_register(GameModeContext *self, pid_t client, pid_t reques } game_mode_apply_client_optimisations(self, client); + + /* Unlock now we're done applying optimisations */ pthread_rwlock_unlock(&self->rwlock); game_mode_client_count_changed(); @@ -516,8 +518,6 @@ int game_mode_context_unregister(GameModeContext *self, pid_t client, pid_t requ break; } - /* Unlock here, potentially yielding */ - if (!found) { LOG_HINTED( ERROR, @@ -538,6 +538,7 @@ int game_mode_context_unregister(GameModeContext *self, pid_t client, pid_t requ game_mode_remove_client_optimisations(self, client); + /* Unlock now we're done applying optimisations */ pthread_rwlock_unlock(&self->rwlock); game_mode_client_count_changed();