Commit cff96883 authored by Tomasz Zawadzki's avatar Tomasz Zawadzki
Browse files

lib/event: remove is_scheduling flag from reactor



There is only one g_scheduling_reactor (main core), the is_scheduling
flag for it is used to block starting new gather_metrics before
previous one is finished.

Meanwhile is_scheduling flag on other reactors was used to block
destroying lw_threads while scheduling happens. It was only needed
because scheduler interacted with the same lw_thread pointers as
each reactor. Previous patch removed this dependency, instead
spdk_thread ids is used. If an spdk_thread is destroyed,
while scheduling _threads_reschedule_thread() handles it.

It is no longer required to block destruction of lw_threads
based on this flag.

Instead of using the main core reactor flag, a g_scheduling_in_progress
is introduced.

Removed _spdk_get_scheduling_reactor() and instead shared the value
of g_scheduling_in_progress between reactor.c and app.c.

Signed-off-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: Ica57326a552477add522174cc3e96b3bab918350
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8732


Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent b74b6133
Loading
Loading
Loading
Loading
+2 −3
Original line number Diff line number Diff line
@@ -88,8 +88,7 @@ struct spdk_reactor {

	struct {
		uint32_t				is_valid : 1;
		uint32_t				is_scheduling : 1;
		uint32_t				reserved : 30;
		uint32_t				reserved : 31;
	} flags;

	uint64_t					tsc_last;
@@ -125,7 +124,7 @@ void spdk_reactors_stop(void *arg1);

struct spdk_reactor *spdk_reactor_get(uint32_t lcore);

struct spdk_reactor *_spdk_get_scheduling_reactor(void);
extern bool g_scheduling_in_progress;

/**
 * Allocate and pass an event to each reactor, serially.
+1 −1
Original line number Diff line number Diff line
@@ -595,7 +595,7 @@ spdk_app_fini(void)
static void
_start_subsystem_fini(void *arg1)
{
	if (_spdk_get_scheduling_reactor()->flags.is_scheduling) {
	if (g_scheduling_in_progress) {
		spdk_thread_send_msg(g_app_thread, _start_subsystem_fini, NULL);
		return;
	}
+9 −30
Original line number Diff line number Diff line
@@ -69,6 +69,7 @@ TAILQ_HEAD(, spdk_scheduler) g_scheduler_list
static struct spdk_scheduler *g_scheduler;
static struct spdk_scheduler *g_new_scheduler;
static struct spdk_reactor *g_scheduling_reactor;
bool g_scheduling_in_progress = false;
static uint64_t g_scheduler_period;
static uint32_t g_scheduler_core_number;
static struct spdk_scheduler_core_info *g_core_infos = NULL;
@@ -112,7 +113,7 @@ _spdk_scheduler_set(char *name)
		return -ENOENT;
	}

	if (g_scheduling_reactor->flags.is_scheduling) {
	if (g_scheduling_in_progress) {
		if (g_scheduler != g_new_scheduler) {
			/* Scheduler already changed, cannot defer multiple deinits */
			return -EBUSY;
@@ -233,12 +234,6 @@ spdk_reactor_get(uint32_t lcore)
	return reactor;
}

struct spdk_reactor *
_spdk_get_scheduling_reactor(void)
{
	return g_scheduling_reactor;
}

static int reactor_thread_op(struct spdk_thread *thread, enum spdk_thread_op op);
static bool reactor_thread_op_supported(enum spdk_thread_op op);

@@ -726,17 +721,10 @@ _threads_reschedule(struct spdk_scheduler_core_info *cores_info)
static void
_reactors_scheduler_fini(void)
{
	struct spdk_reactor *reactor;
	uint32_t i;

	/* Reschedule based on the balancing output */
	_threads_reschedule(g_core_infos);

	SPDK_ENV_FOREACH_CORE(i) {
		reactor = spdk_reactor_get(i);
		assert(reactor != NULL);
		reactor->flags.is_scheduling = false;
	}
	g_scheduling_in_progress = false;
}

static void
@@ -784,14 +772,7 @@ _reactors_scheduler_balance(void *arg1, void *arg2)
static void
_reactors_scheduler_cancel(void *arg1, void *arg2)
{
	struct spdk_reactor *reactor;
	uint32_t i;

	SPDK_ENV_FOREACH_CORE(i) {
		reactor = spdk_reactor_get(i);
		assert(reactor != NULL);
		reactor->flags.is_scheduling = false;
	}
	g_scheduling_in_progress = false;
}

/* Phase 1 of thread scheduling is to gather metrics on the existing threads */
@@ -808,7 +789,6 @@ _reactors_scheduler_gather_metrics(void *arg1, void *arg2)

	reactor = spdk_reactor_get(spdk_env_get_current_core());
	assert(reactor != NULL);
	reactor->flags.is_scheduling = true;
	core_info = &g_core_infos[reactor->lcore];
	core_info->lcore = reactor->lcore;
	core_info->current_idle_tsc = reactor->idle_tsc - core_info->total_idle_tsc;
@@ -904,12 +884,10 @@ reactor_post_process_lw_thread(struct spdk_reactor *reactor, struct spdk_lw_thre

	if (spdk_unlikely(spdk_thread_is_exited(thread) &&
			  spdk_thread_is_idle(thread))) {
		if (reactor->flags.is_scheduling == false) {
		_reactor_remove_lw_thread(reactor, lw_thread);
		spdk_thread_destroy(thread);
		return true;
	}
	}

	return false;
}
@@ -995,7 +973,7 @@ reactor_run(void *arg)
		if (spdk_unlikely(g_scheduler_period > 0 &&
				  (reactor->tsc_last - last_sched) > g_scheduler_period &&
				  reactor == g_scheduling_reactor &&
				  !reactor->flags.is_scheduling)) {
				  !g_scheduling_in_progress)) {
			if (spdk_unlikely(g_scheduler != g_new_scheduler)) {
				if (g_scheduler->deinit != NULL) {
					g_scheduler->deinit(&g_governor);
@@ -1005,6 +983,7 @@ reactor_run(void *arg)

			if (spdk_unlikely(g_scheduler->balance != NULL)) {
				last_sched = reactor->tsc_last;
				g_scheduling_in_progress = true;
				_reactors_scheduler_gather_metrics(NULL, NULL);
			}
		}
+1 −1
Original line number Diff line number Diff line
@@ -59,7 +59,7 @@ DEFINE_STUB_V(spdk_reactors_stop, (void *arg1));
DEFINE_STUB(spdk_reactors_init, int, (void), 0);
DEFINE_STUB_V(spdk_reactors_fini, (void));
DEFINE_STUB_V(_spdk_scheduler_disable, (void));
DEFINE_STUB(_spdk_get_scheduling_reactor, struct spdk_reactor *, (void), NULL);
bool g_scheduling_in_progress;

static void
unittest_usage(void)