sched: only log when scheduler type changes

Closes 23552. Thanks dgoulet for original impl

changes/bug23552

@@ -0,0 +1,3 @@
+  o Minor bugfixes (scheduler):
+    - Only notice log the selected scheduler when we switch scheduler types.
+      Fixes bug 23552; bugfix on 0.3.2.1-alpha.

src/or/scheduler.c

@@ -174,6 +174,25 @@ STATIC struct event *run_sched_ev = NULL;
  * Functions that can only be accessed from this file.
  *****************************************************************************/
 
+/** Return a human readable string for the given scheduler type. */
+static const char *
+get_scheduler_type_string(scheduler_types_t type)
+{
+  switch(type) {
+  case SCHEDULER_VANILLA:
+    return "Vanilla";
+  case SCHEDULER_KIST:
+    return "KIST";
+  case SCHEDULER_KIST_LITE:
+    return "KISTLite";
+  case SCHEDULER_NONE:
+    /* fallthrough */
+  default:
+    tor_assert_unreached();
+    return "(N/A)";
+  }
+}
+
 /**
  * Scheduler event callback; this should get triggered once per event loop
  * if any scheduling work was created during the event loop.
@@ -312,6 +331,8 @@ select_scheduler(void)
       new_scheduler = get_kist_scheduler();
       scheduler_kist_set_lite_mode();
       goto end;
+    case SCHEDULER_NONE:
+      /* fallthrough */
     default:
       /* Our option validation should have caught this. */
       tor_assert_unreached();
@@ -333,8 +354,6 @@ select_scheduler(void)
 
   /* Set the chosen scheduler. */
   the_scheduler = new_scheduler;
-  log_notice(LD_CONFIG, "Scheduler type %s has been enabled.",
-             chosen_sched_type);
 }
 
 /**
@@ -346,11 +365,21 @@ static void
 set_scheduler(void)
 {
   const scheduler_t *old_scheduler = the_scheduler;
+  scheduler_types_t old_scheduler_type = SCHEDULER_NONE;
+
+  /* We keep track of the type in order to log only if the type switched. We
+   * can't just use the scheduler pointers because KIST and KISTLite share the
+   * same object. */
+  if (the_scheduler) {
+    old_scheduler_type = the_scheduler->type;
+  }
 
   /* From the options, select the scheduler type to set. */
   select_scheduler();
   tor_assert(the_scheduler);
 
+  /* We look at the pointer difference in case the old sched and new sched
+   * share the same scheduler object, as is the case with KIST and KISTLite. */
   if (old_scheduler != the_scheduler) {
     /* Allow the old scheduler to clean up, if needed. */
     if (old_scheduler && old_scheduler->free_all) {
@@ -362,6 +391,13 @@ set_scheduler(void)
       the_scheduler->init();
     }
   }
+
+  /* Finally we notice log if we switched schedulers. We use the type in case
+   * two schedulers share a scheduler object. */
+  if (old_scheduler_type != the_scheduler->type) {
+    log_notice(LD_CONFIG, "Scheduler type %s has been enabled.",
+               get_scheduler_type_string(the_scheduler->type));
+  }
 }
 
 /**

src/or/scheduler.h

@@ -13,7 +13,17 @@
 #include "channel.h"
 #include "testsupport.h"
 
-/*
+/** Scheduler type, we build an ordered list with those values from the
+ * parsed strings in Schedulers. The reason to do such a thing is so we can
+ * quickly and without parsing strings select the scheduler at anytime. */
+typedef enum {
+  SCHEDULER_NONE =     -1,
+  SCHEDULER_VANILLA =   1,
+  SCHEDULER_KIST =      2,
+  SCHEDULER_KIST_LITE = 3,
+} scheduler_types_t;
+
+/**
  * A scheduler implementation is a collection of function pointers. If you
  * would like to add a new scheduler called foo, create scheduler_foo.c,
  * implement at least the mandatory ones, and implement get_foo_scheduler()
@@ -31,6 +41,10 @@
  * is shutting down), then set that function pointer to NULL.
  */
 typedef struct scheduler_s {
+  /* Scheduler type. This is used for logging when the scheduler is switched
+   * during runtime. */
+  scheduler_types_t type;
+
   /* (Optional) To be called when we want to prepare a scheduler for use.
    * Perhaps Tor just started and we are the lucky chosen scheduler, or
    * perhaps Tor is switching to this scheduler. No matter the case, this is
@@ -82,15 +96,6 @@ typedef struct scheduler_s {
   void (*on_new_options)(void);
 } scheduler_t;
 
-/** Scheduler type, we build an ordered list with those values from the
- * parsed strings in Schedulers. The reason to do such a thing is so we can
- * quickly and without parsing strings select the scheduler at anytime. */
-typedef enum {
-  SCHEDULER_VANILLA =   1,
-  SCHEDULER_KIST =      2,
-  SCHEDULER_KIST_LITE = 3,
-} scheduler_types_t;
-
 /*****************************************************************************
  * Globally visible scheduler variables/values
  *

src/or/scheduler_kist.c

@@ -687,6 +687,7 @@ kist_scheduler_run(void)
 
 /* Stores the kist scheduler function pointers. */
 static scheduler_t kist_scheduler = {
+  .type = SCHEDULER_KIST,
   .free_all = kist_free_all,
   .on_channel_free = kist_on_channel_free,
   .init = kist_scheduler_init,
@@ -738,6 +739,7 @@ void
 scheduler_kist_set_lite_mode(void)
 {
   kist_lite_mode = 1;
+  kist_scheduler.type = SCHEDULER_KIST_LITE;
   log_info(LD_SCHED,
            "Setting KIST scheduler without kernel support (KISTLite mode)");
 }
@@ -747,6 +749,7 @@ void
 scheduler_kist_set_full_mode(void)
 {
   kist_lite_mode = 0;
+  kist_scheduler.type = SCHEDULER_KIST;
   log_info(LD_SCHED,
            "Setting KIST scheduler with kernel support (KIST mode)");
 }

src/or/scheduler_vanilla.c

@@ -179,6 +179,7 @@ vanilla_scheduler_run(void)
 
 /* Stores the vanilla scheduler function pointers. */
 static scheduler_t vanilla_scheduler = {
+  .type = SCHEDULER_VANILLA,
   .free_all = NULL,
   .on_channel_free = NULL,
   .init = NULL,