Merge pull request #144 from mdiluz/add-config-hotreloading

Add config hot-reloading

daemon/daemon_config.c

@@ -37,11 +37,14 @@ POSSIBILITY OF SUCH DAMAGE.
 /* Ben Hoyt's inih library */
 #include "ini.h"
 
+#include <dirent.h>
 #include <linux/limits.h>
 #include <pthread.h>
 #include <pwd.h>
 #include <stdio.h>
 #include <string.h>
+#include <sys/inotify.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 
 /* 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);

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

daemon/dbus_messaging.c

@@ -206,24 +206,35 @@ 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_reload_config(context);
+	return sd_bus_reply_method_return(m, "i", status);
+}
+
 /**
  * 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_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.

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;
@@ -410,21 +436,16 @@ 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) {
 		game_mode_context_enter(self);
 	}
 
-	/* Store current renice and apply */
-	game_mode_apply_renice(self, client, 0 /* expect zero value to start with */);
+	game_mode_apply_client_optimisations(self, client);
 
-	/* Store current ioprio value and apply  */
-	game_mode_apply_ioprio(self, client, IOPRIO_DEFAULT);
-
-	/* Apply scheduler policies */
-	game_mode_apply_scheduling(self, client);
+	/* Unlock now we're done applying optimisations */
+	pthread_rwlock_unlock(&self->rwlock);
 
 	game_mode_client_count_changed();
 
@@ -438,6 +459,17 @@ error_cleanup:
 	return err;
 }
 
+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;
@@ -491,9 +523,6 @@ int game_mode_context_unregister(GameModeContext *self, pid_t client, pid_t requ
 		break;
 	}
 
-	/* Unlock here, potentially yielding */
-	pthread_rwlock_unlock(&self->rwlock);
-
 	if (!found) {
 		LOG_HINTED(
 		    ERROR,
@@ -503,6 +532,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;
 	}
 
@@ -511,13 +541,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);
 
-	/* 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));
+	/* Unlock now we're done applying optimisations */
+	pthread_rwlock_unlock(&self->rwlock);
 
-	/* 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_client_count_changed();
 
 	return 0;
 }
@@ -613,6 +642,39 @@ 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");
+
+	/* Make sure we have a readwrite lock on ourselves */
+	pthread_rwlock_wrlock(&self->rwlock);
+
+	/* 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);
+
+	/* Re-apply all current optimisations */
+	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");
+}
+
 /**
  * We continuously run until told otherwise.
  */
@@ -640,6 +702,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;
 	}
 
@@ -736,3 +804,23 @@ static void game_mode_execute_scripts(char scripts[CONFIG_LIST_MAX][CONFIG_VALUE
 		i++;
 	}
 }
+
+/*
+ * Reload the current configuration
+ *
+ * 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_reload_config(GameModeContext *self)
+{
+	/* Stop the reaper thread first */
+	end_reaper_thread(self);
+
+	game_mode_reload_config_internal(self);
+
+	/* Restart the reaper thread back up again */
+	start_reaper_thread(self);
+
+	return 0;
+}

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);
 
+/*
+ * Reload the current configuration
+ */
+int game_mode_reload_config(GameModeContext *context);
+
 /** gamemode-env.c
  * Provides internal API functions specific to working environment
  * variables.

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"