[gnome-software: 2/9] gs-ioprio: Accept a priority argument to gs_ioprio_init()




commit c16e244388e996b7ffd895385c136a5c51ae882c
Author: Philip Withnall <pwithnall endlessos org>
Date:   Mon Mar 28 15:20:14 2022 +0100

    gs-ioprio: Accept a priority argument to gs_ioprio_init()
    
    This pivots the function from being a one-time call at the time of
    creating a thread, which lowers the thread’s priority; to something
    which can be called at any point to lower or raise the thread’s
    priority.
    
    Previously, the threads in the `GsPluginLoader` thread pool could be set
    to low I/O priority because (in theory) they were only used for
    non-interactive jobs, and all non-interactive jobs were done in those
    threads. See commit 59b40d256.  (That doesn’t actually reflect the code
    though: all install/update/upgrade-download jobs were done in these
    low-priority threads, whether interactive or not; and no other jobs were
    done in these threads, whether interactive or not.)
    
    That approach doesn’t match the new threading model of having one worker
    thread per plugin (if the plugin wants one). In the new threading model,
    both interactive and non-interactive jobs are executed on the same
    worker thread.
    
    This means the I/O priority of that worker thread needs to change over
    time, being low for non-interactive jobs, and default for interactive
    jobs.
    
    This should mean that gnome-software doesn’t grind the disk in the
    background when doing updates, and conversely doesn’t take forever to
    install an app in the foreground.
    
    This commit renames `gs_ioprio_init()` to `gs_ioprio_set()`, but keeps
    the existing behaviour at all its call sites. Following commits will
    change the behaviour for worker threads. The behaviour for thread pool
    threads will not be changed (i.e. they will remain low I/O priority
    throughout their lives), as they will eventually be superseded by the
    new threading model, so there’s no point in thinking deeply about
    whether it’s safe to change their I/O priority behaviour.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>

 lib/gs-ioprio.c        | 71 +++++++++++++++++++++++++++++++++++++++++++++-----
 lib/gs-ioprio.h        |  2 +-
 lib/gs-plugin-loader.c |  2 +-
 3 files changed, 67 insertions(+), 8 deletions(-)
---
diff --git a/lib/gs-ioprio.c b/lib/gs-ioprio.c
index 97f231c63..3a63d078b 100644
--- a/lib/gs-ioprio.c
+++ b/lib/gs-ioprio.c
@@ -116,14 +116,73 @@ set_io_priority (int ioprio,
        return ioprio_set (IOPRIO_WHO_PROCESS, 0, ioprio | (ioclass << IOPRIO_CLASS_SHIFT));
 }
 
+static const gchar *
+ioclass_to_string (int ioclass)
+{
+       switch (ioclass) {
+       case IOPRIO_CLASS_IDLE:
+               return "IDLE";
+       case IOPRIO_CLASS_BE:
+               return "BE";
+       default:
+               return "unknown";
+       }
+}
+
+/**
+ * gs_ioprio_set:
+ * @priority: I/O priority, with higher numeric values indicating lower priority;
+ *   use %G_PRIORITY_DEFAULT as the default
+ *
+ * Set the I/O priority of the current thread using the `ioprio_set()` syscall.
+ *
+ * The @priority is quantised before being passed to the kernel.
+ *
+ * This function may fail if the process doesn’t have permission to change its
+ * I/O priority to the given value. If so, a warning will be printed, as the
+ * quantised priority values are chosen so they shouldn’t typically require
+ * permissions to set.
+ */
 void
-gs_ioprio_init (void)
+gs_ioprio_set (gint priority)
 {
-       if (set_io_priority (7, IOPRIO_CLASS_IDLE) == -1) {
-               g_message ("Could not set idle IO priority, attempting best effort of 7");
+       int ioprio, ioclass;
+
+       /* If the priority is lower than default, use an idle I/O priority. The
+        * condition looks wrong because higher integers indicate lower priority
+        * in GLib.
+        *
+        * Otherwise use a default best-effort priority, which is the same as
+        * what all new threads get (in the absence of an I/O context with
+        * `CLONE_IO`). */
+       if (priority > G_PRIORITY_DEFAULT) {
+               ioprio = 7;
+               ioclass = IOPRIO_CLASS_IDLE;
+       } else if (priority == G_PRIORITY_DEFAULT) {
+               ioprio = 4;  /* this is the default priority in the BE class */
+               ioclass = IOPRIO_CLASS_BE;
+       } else {
+               ioprio = 0;  /* this is the highest priority in the BE class */
+               ioclass = IOPRIO_CLASS_BE;
+       }
+
+       g_debug ("Setting I/O priority of thread %p to %s, %d",
+                g_thread_self (), ioclass_to_string (ioclass), ioprio);
+
+       if (set_io_priority (ioprio, ioclass) == -1) {
+               g_warning ("Could not set I/O priority to %s, %d",
+                          ioclass_to_string (ioclass), ioprio);
+
+               /* If we were trying to set to idle priority, try again with the
+                * lowest-possible best-effort priority. This is because kernels
+                * older than 2.6.25 required `CAP_SYS_ADMIN` to set
+                * `IOPRIO_CLASS_IDLE`. Newer kernels do not. */
+               if (ioclass == IOPRIO_CLASS_IDLE) {
+                       ioprio = 7;  /* this is the lowest priority in the BE class */
+                       ioclass = IOPRIO_CLASS_BE;
 
-               if (set_io_priority (7, IOPRIO_CLASS_BE) == -1) {
-                       g_message ("Could not set best effort IO priority either, giving up");
+                       if (set_io_priority (ioprio, ioclass) == -1)
+                               g_warning ("Could not set best effort IO priority either, giving up");
                }
        }
 }
@@ -131,7 +190,7 @@ gs_ioprio_init (void)
 #else  /* __linux__ */
 
 void
-gs_ioprio_init (void)
+gs_ioprio_set (gint priority)
 {
 }
 
diff --git a/lib/gs-ioprio.h b/lib/gs-ioprio.h
index b876afe3b..e630b8ab5 100644
--- a/lib/gs-ioprio.h
+++ b/lib/gs-ioprio.h
@@ -26,6 +26,6 @@
 
 G_BEGIN_DECLS
 
-void gs_ioprio_init (void);
+void gs_ioprio_set (gint priority);
 
 G_END_DECLS
diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c
index 3bc93b8cb..3531489cc 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -3507,7 +3507,7 @@ gs_plugin_loader_process_in_thread_pool_cb (gpointer data,
        GsApp *app = gs_plugin_job_get_app (helper->plugin_job);
        GsPluginAction action = gs_plugin_job_get_action (helper->plugin_job);
 
-       gs_ioprio_init ();
+       gs_ioprio_set (G_PRIORITY_LOW);
 
        gs_plugin_loader_process_thread_cb (task, source_object, task_data, cancellable);
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]