From d6c4f8cf6b6d095b0b6efed9e6c72af52fa30548 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Thu, 15 Jul 2021 09:21:24 -0400 Subject: [PATCH] lib/event: refactor _spdk_scheduler_set() By default g_scheduler is now NULL. It can be set either by event framework or RPC. To keep RPC consistent, the 'static' scheduler was kept. All access to the g_scheduler is done via _spdk_scheduler_get(). Access to its members is done to balance, get name for RPC and through _spdk_scheduler_set(). All of them happen on scheduling reactor and don't pose race condition when changing scheduler. There is no need to delay init/deinit of scheduler via g_new_scheduler. This variable was removed and all that happens in _spdk_scheduler_set(). To unset and deinitialize current scheduler, _spdk_scheduler_set(NULL) has to be called. This results in moving scheduler deinitalization to that call too. Every spdk_scheduler callback is now mandatory. Signed-off-by: Tomasz Zawadzki Change-Id: I1fed766989de9bcaeb7e82b490c1275f3572d77d Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8811 Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot Community-CI: Broadcom CI Reviewed-by: Ben Walker Reviewed-by: Konrad Sztyber Reviewed-by: Jim Harris --- include/spdk_internal/event.h | 5 ++- lib/event/reactor.c | 62 ++++++++++++++++------------------- lib/event/scheduler_static.c | 22 +++++++++++-- 3 files changed, 51 insertions(+), 38 deletions(-) diff --git a/include/spdk_internal/event.h b/include/spdk_internal/event.h index 3989f995f..e41690e04 100644 --- a/include/spdk_internal/event.h +++ b/include/spdk_internal/event.h @@ -291,7 +291,10 @@ struct spdk_scheduler { void _spdk_scheduler_list_add(struct spdk_scheduler *scheduler); /** - * Change current scheduler. + * Change current scheduler. If another scheduler was used prior, + * it will be deinitalized. No scheduler will be set if name parameter + * is NULL. + * This function should be invoked from scheduling reactor. * * \param name Name of the scheduler to be used. * diff --git a/lib/event/reactor.c b/lib/event/reactor.c index 4fa1d9421..b11c6dc01 100644 --- a/lib/event/reactor.c +++ b/lib/event/reactor.c @@ -66,8 +66,7 @@ static struct spdk_mempool *g_spdk_event_mempool = NULL; TAILQ_HEAD(, spdk_scheduler) g_scheduler_list = TAILQ_HEAD_INITIALIZER(g_scheduler_list); -static struct spdk_scheduler *g_scheduler; -static struct spdk_scheduler *g_new_scheduler; +static struct spdk_scheduler *g_scheduler = NULL; static struct spdk_reactor *g_scheduling_reactor; bool g_scheduling_in_progress = false; static uint64_t g_scheduler_period; @@ -100,32 +99,36 @@ int _spdk_scheduler_set(char *name) { struct spdk_scheduler *scheduler; + int rc = 0; + + /* NULL scheduler was specifically requested */ + if (name == NULL) { + if (g_scheduler) { + g_scheduler->deinit(); + } + g_scheduler = NULL; + return 0; + } scheduler = _scheduler_find(name); if (scheduler == NULL) { SPDK_ERRLOG("Requested scheduler is missing\n"); - return -ENOENT; + return -EINVAL; } - if (g_scheduling_in_progress) { - if (g_scheduler != g_new_scheduler) { - /* Scheduler already changed, cannot defer multiple deinits */ - return -EBUSY; - } - } else { - if (g_scheduler != NULL && g_scheduler->deinit != NULL) { + if (g_scheduler == scheduler) { + return 0; + } + + rc = scheduler->init(); + if (rc == 0) { + if (g_scheduler) { g_scheduler->deinit(); } g_scheduler = scheduler; } - g_new_scheduler = scheduler; - - if (scheduler->init != NULL) { - scheduler->init(); - } - - return 0; + return rc; } struct spdk_scheduler * @@ -299,9 +302,7 @@ spdk_reactors_fini(void) return; } - if (g_scheduler->deinit != NULL) { - g_scheduler->deinit(); - } + _spdk_scheduler_set(NULL); spdk_thread_lib_fini(); @@ -750,8 +751,10 @@ _reactors_scheduler_update_core_mode(void *ctx) static void _reactors_scheduler_balance(void *arg1, void *arg2) { - if (g_reactor_state == SPDK_REACTOR_STATE_RUNNING) { - g_scheduler->balance(g_core_infos, g_reactor_count); + struct spdk_scheduler *scheduler = _spdk_scheduler_get(); + + if (g_reactor_state == SPDK_REACTOR_STATE_RUNNING && scheduler != NULL) { + scheduler->balance(g_core_infos, g_reactor_count); g_scheduler_core_number = spdk_env_get_first_core(); _reactors_scheduler_update_core_mode(NULL); @@ -956,18 +959,9 @@ reactor_run(void *arg) (reactor->tsc_last - last_sched) > g_scheduler_period && reactor == g_scheduling_reactor && !g_scheduling_in_progress)) { - if (spdk_unlikely(g_scheduler != g_new_scheduler)) { - if (g_scheduler->deinit != NULL) { - g_scheduler->deinit(); - } - g_scheduler = g_new_scheduler; - } - - if (spdk_unlikely(g_scheduler->balance != NULL)) { - last_sched = reactor->tsc_last; - g_scheduling_in_progress = true; - _reactors_scheduler_gather_metrics(NULL, NULL); - } + last_sched = reactor->tsc_last; + g_scheduling_in_progress = true; + _reactors_scheduler_gather_metrics(NULL, NULL); } if (g_reactor_state != SPDK_REACTOR_STATE_RUNNING) { diff --git a/lib/event/scheduler_static.c b/lib/event/scheduler_static.c index c585a444a..db9f34d89 100644 --- a/lib/event/scheduler_static.c +++ b/lib/event/scheduler_static.c @@ -39,10 +39,26 @@ #include "spdk_internal/event.h" +static int +init_static(void) +{ + return 0; +} + +static void +deinit_static(void) +{ +} + +static void +balance_static(struct spdk_scheduler_core_info *cores, int core_count) +{ +} + static struct spdk_scheduler scheduler = { .name = "static", - .init = NULL, - .deinit = NULL, - .balance = NULL, + .init = init_static, + .deinit = deinit_static, + .balance = balance_static, }; SPDK_SCHEDULER_REGISTER(scheduler);