lib/event: refactor _spdk_governor_set()

By default g_governor is now NULL. It can be set
either by event framework or schedulers directly.

Dynamic_scheduler and gscheduler specifically want
to use the dpdk_governor, so their initialization
now sets it explicitly.

To unset and deinitialize current governor,
_spdk_governor_set(NULL) has to be called.
This results in moving governor deinitalization to that
call too.

The "default" governor has been removed.

Every spdk_governor callback is now mandatory.

Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: Ibf76bd28bfbb159416026996fa217bb3325a3d31
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8810
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@gmail.com>

include/spdk_internal/event.h

@@ -198,7 +198,11 @@ struct spdk_governor {
 void _spdk_governor_list_add(struct spdk_governor *governor);
 
 /**
- * Change current governor.
+ * Set the current governor.
+ *
+ * Always deinitalizes previously set governor.
+ * No governor will be set if name parameter is NULL.
+ * This function should be invoked on scheduling reactor.
  *
  * \param name Name of the governor to be used.
  *

lib/event/app_rpc.c

@@ -400,8 +400,7 @@ _rpc_framework_get_reactors(void *arg1, void *arg2)
 	spdk_json_write_named_uint64(ctx->w, "idle", reactor->idle_tsc);
 	spdk_json_write_named_bool(ctx->w, "in_interrupt", reactor->in_interrupt);
 	governor = _spdk_governor_get();
-	/* We need to check whether governor can return current core frequency. */
-	if (governor->get_core_curr_freq != NULL) {
+	if (governor != NULL) {
 		/* Governor returns core freqs in kHz, we want MHz. */
 		curr_core_freq = governor->get_core_curr_freq(current_core) / 1000;
 		spdk_json_write_named_uint32(ctx->w, "core_freq", curr_core_freq);
@@ -528,7 +527,9 @@ rpc_framework_get_scheduler(struct spdk_jsonrpc_request *request,
 	spdk_json_write_object_begin(w);
 	spdk_json_write_named_string(w, "scheduler_name", scheduler->name);
 	spdk_json_write_named_uint64(w, "scheduler_period", scheduler_period);
-	spdk_json_write_named_string(w, "governor_name", governor->name);
+	if (governor != NULL) {
+		spdk_json_write_named_string(w, "governor_name", governor->name);
+	}
 	spdk_json_write_object_end(w);
 	spdk_jsonrpc_end_result(request, w);
 }

lib/event/gscheduler.c

@@ -49,12 +49,7 @@ init(void)
 static void
 deinit(void)
 {
-	struct spdk_governor *governor;
-
-	governor = _spdk_governor_get();
-	if (governor->deinit) {
-		governor->deinit();
-	}
+	_spdk_governor_set(NULL);
 }
 
 static void
@@ -67,6 +62,8 @@ balance(struct spdk_scheduler_core_info *cores, int core_count)
 	int rc;
 
 	governor = _spdk_governor_get();
+	assert(governor != NULL);
+
 	/* Gather active/idle statistics */
 	SPDK_ENV_FOREACH_CORE(i) {
 		core = &cores[i];

lib/event/reactor.c

@@ -77,13 +77,7 @@ static struct spdk_scheduler_core_info *g_core_infos = NULL;
 TAILQ_HEAD(, spdk_governor) g_governor_list
 	= TAILQ_HEAD_INITIALIZER(g_governor_list);
 
-static int _governor_get_capabilities(uint32_t lcore_id,
-				      struct spdk_governor_capabilities *capabilities);
-
-static struct spdk_governor g_governor = {
-	.name = "default",
-	.get_core_capabilities = _governor_get_capabilities,
-};
+static struct spdk_governor *g_governor;
 
 static int reactor_interrupt_init(struct spdk_reactor *reactor);
 static void reactor_interrupt_fini(struct spdk_reactor *reactor);
@@ -1477,14 +1471,6 @@ reactor_interrupt_fini(struct spdk_reactor *reactor)
 	reactor->fgrp = NULL;
 }
 
-static int
-_governor_get_capabilities(uint32_t lcore_id, struct spdk_governor_capabilities *capabilities)
-{
-	capabilities->priority = false;
-
-	return 0;
-}
-
 static struct spdk_governor *
 _governor_find(char *name)
 {
@@ -1505,25 +1491,39 @@ _spdk_governor_set(char *name)
 	struct spdk_governor *governor;
 	int rc = 0;
 
+	/* NULL governor was specifically requested */
+	if (name == NULL) {
+		if (g_governor) {
+			g_governor->deinit();
+		}
+		g_governor = NULL;
+		return 0;
+	}
+
 	governor = _governor_find(name);
 	if (governor == NULL) {
 		return -EINVAL;
 	}
 
-	if (governor->init) {
-		rc = governor->init();
+	if (g_governor == governor) {
+		return 0;
 	}
 
+	rc = governor->init();
 	if (rc == 0) {
-		g_governor = *governor;
+		if (g_governor) {
+			g_governor->deinit();
+		}
+		g_governor = governor;
 	}
+
 	return rc;
 }
 
 struct spdk_governor *
 _spdk_governor_get(void)
 {
-	return &g_governor;
+	return g_governor;
 }
 
 void

lib/event/scheduler_dynamic.c

@@ -242,15 +242,10 @@ init(void)
 static void
 deinit(void)
 {
-	struct spdk_governor *governor;
-
 	free(g_cores);
 	g_cores = NULL;
 
-	governor = _spdk_governor_get();
-	if (governor->deinit) {
-		governor->deinit();
-	}
+	_spdk_governor_set(NULL);
 }
 
 static void
@@ -324,6 +319,8 @@ balance(struct spdk_scheduler_core_info *cores_info, int cores_count)
 	}
 
 	governor = _spdk_governor_get();
+	assert(governor != NULL);
+
 	/* Change main core frequency if needed */
 	if (busy_threads_present) {
 		rc = governor->set_core_freq_max(g_main_lcore);

test/unit/lib/event/reactor.c/reactor_ut.c

@@ -762,16 +762,22 @@ core_freq_max(uint32_t lcore)
 	return 0;
 }
 
+DEFINE_STUB(core_freq_min, int, (uint32_t lcore_id), 0);
+DEFINE_STUB(core_caps, int,
+	    (uint32_t lcore_id, struct spdk_governor_capabilities *capabilities), 0);
+DEFINE_STUB(governor_init, int, (void), 0);
+DEFINE_STUB_V(governor_deinit, (void));
+
 static struct spdk_governor governor = {
 	.name = "dpdk_governor",
 	.get_core_curr_freq = NULL,
 	.core_freq_up = core_freq_up,
 	.core_freq_down = core_freq_down,
 	.set_core_freq_max = core_freq_max,
-	.set_core_freq_min = NULL,
-	.get_core_capabilities = NULL,
-	.init = NULL,
-	.deinit = NULL,
+	.set_core_freq_min = core_freq_min,
+	.get_core_capabilities = core_caps,
+	.init = governor_init,
+	.deinit = governor_deinit,
 };
 
 static void