Commit 11ff66fe authored by syeon.shin's avatar syeon.shin Committed by Jim Harris
Browse files

lib/event: Modify spdk_reactor_set_interrupt_mode() to be called from scheduling reactor 



The previous behavior of spdk_reactor_set_interrupt_mode()
returning an error when called from a non-app_thread context
has been changed to return an error when called from a
non-scheduling reactor context. As a result,
spdk_reactor_set_interrupt_mode() must now be executed from a
scheduling core rather than an app thread. Modifications have
been made to address instances where this requirement was not met.

There is no issue when the app thread operates on the scheduling
core, but if it runs on a different core, changing the interrupt
mode will result in an error stating that it can only be modified
on the scheduling core. This could cause scheduling operations
to malfunction. To address this, modifications have been made.

Change-Id: I1f73c84191f5cab877a29405756f106cfb410932
Signed-off-by: default avatarsyeon.shin <syeon.shin@samsung.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/24324


Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
parent 78467a35

examples/interrupt_tgt/interrupt_tgt.c

+3 −3
Original line number Diff line number Diff line
@@ -40,9 +40,9 @@ rpc_reactor_set_interrupt_mode_cb(void *cb_arg) 
}



static void

set_interrupt_mode_cb(void *arg)

set_interrupt_mode_cb(void *arg1, void *arg2)

{

	struct rpc_reactor_set_interrupt_mode *req = arg;

	struct rpc_reactor_set_interrupt_mode *req = arg1;



	spdk_thread_send_msg(req->rpc_thread, rpc_reactor_set_interrupt_mode_cb, req);

}
@@ -57,7 +57,7 @@ set_interrupt_mode(void *arg1, void *arg2) 
					     set_interrupt_mode_cb, req);

	if (rc)	{

		req->rc = rc;

		set_interrupt_mode_cb(req);

		set_interrupt_mode_cb(req, NULL);

	}

}

include/spdk_internal/event.h

+1 −1
Original line number Diff line number Diff line
@@ -48,7 +48,7 @@ struct spdk_lw_thread { 
 *

 * \param cb_arg Argument to pass to the callback function.

 */

typedef void (*spdk_reactor_set_interrupt_mode_cb)(void *cb_arg);

typedef void (*spdk_reactor_set_interrupt_mode_cb)(void *cb_arg1, void *cb_arg2);



struct spdk_reactor {

	/* Lightweight threads running on this reactor */

lib/event/reactor.c

+7 −7
Original line number Diff line number Diff line
@@ -352,8 +352,8 @@ _reactor_set_notify_cpuset_cpl(void *arg1, void *arg2) 


	if (target->new_in_interrupt == false) {

		target->set_interrupt_mode_in_progress = false;

		spdk_thread_send_msg(spdk_thread_get_app_thread(), target->set_interrupt_mode_cb_fn,

				     target->set_interrupt_mode_cb_arg);

		_event_call(spdk_scheduler_get_scheduling_lcore(), target->set_interrupt_mode_cb_fn,

			    target->set_interrupt_mode_cb_arg, NULL);

	} else {

		_event_call(target->lcore, _reactor_set_interrupt_mode, target, NULL);

	}
@@ -419,8 +419,8 @@ _reactor_set_interrupt_mode(void *arg1, void *arg2) 
		}



		target->set_interrupt_mode_in_progress = false;

		spdk_thread_send_msg(spdk_thread_get_app_thread(), target->set_interrupt_mode_cb_fn,

				     target->set_interrupt_mode_cb_arg);

		_event_call(spdk_scheduler_get_scheduling_lcore(), target->set_interrupt_mode_cb_fn,

			    target->set_interrupt_mode_cb_arg, NULL);

	}

}
@@ -446,7 +446,7 @@ spdk_reactor_set_interrupt_mode(uint32_t lcore, bool new_in_interrupt, 
	}



	if (target->in_interrupt == new_in_interrupt) {

		cb_fn(cb_arg);

		cb_fn(cb_arg, NULL);

		return 0;

	}
@@ -722,7 +722,7 @@ _reactors_scheduler_fini(void) 
}



static void

_reactors_scheduler_update_core_mode(void *ctx)

_reactors_scheduler_update_core_mode(void *ctx1, void *ctx2)

{

	struct spdk_reactor *reactor;

	uint32_t i;
@@ -774,7 +774,7 @@ _reactors_scheduler_balance(void *arg1, void *arg2) 
	scheduler->balance(g_core_infos, g_reactor_count);



	g_scheduler_core_number = spdk_env_get_first_core();

	_reactors_scheduler_update_core_mode(NULL);

	_reactors_scheduler_update_core_mode(NULL, NULL);

}



/* Phase 1 of thread scheduling is to gather metrics on the existing threads */

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

+9 −5
Original line number Diff line number Diff line
@@ -504,7 +504,6 @@ static uint32_t 
_run_events_till_completion(uint32_t reactor_count)

{

	struct spdk_reactor *reactor;

	struct spdk_thread *app_thread = spdk_thread_get_app_thread();

	uint32_t i, events;

	uint32_t total_events = 0;
@@ -516,9 +515,9 @@ _run_events_till_completion(uint32_t reactor_count) 
			MOCK_SET(spdk_env_get_current_core, i);

			events += event_queue_run_batch(reactor);



			/* Some events still require app_thread to run */

			/* Some events require scheduling core to run */

			MOCK_SET(spdk_env_get_current_core, g_scheduling_reactor->lcore);

			spdk_thread_poll(app_thread, 0, 0);

			events += event_queue_run_batch(g_scheduling_reactor);



			MOCK_CLEAR(spdk_env_get_current_core);

		}
@@ -875,6 +874,7 @@ test_bind_thread(void) 
	free_cores();

}



#ifndef __FreeBSD__

uint8_t g_curr_freq;



static int
@@ -1033,7 +1033,7 @@ test_governor(void) 


	i = _run_events_till_completion(2);

	/* Six runs when interrupt mode is supported, two if not. */

	CU_ASSERT(i == 6 || i == 2);

	CU_ASSERT(i == 7 || i == 3);

	MOCK_SET(spdk_env_get_current_core, 0);



	/* Main core should be busy more than 50% time now - frequency should be raised */
@@ -1058,7 +1058,7 @@ test_governor(void) 


	i = _run_events_till_completion(2);

	/* Six runs when interrupt mode is supported, two if not. */

	CU_ASSERT(i == 6 || i == 2);

	CU_ASSERT(i == 7 || i == 3);

	MOCK_SET(spdk_env_get_current_core, 0);



	for (i = 0; i < 2; i++) {
@@ -1091,6 +1091,7 @@ test_governor(void) 


	free_cores();

}

#endif



int

main(int argc, char **argv)
@@ -1111,7 +1112,10 @@ main(int argc, char **argv) 
	CU_ADD_TEST(suite, test_for_each_reactor);

	CU_ADD_TEST(suite, test_reactor_stats);

	CU_ADD_TEST(suite, test_scheduler);

#ifndef __FreeBSD__

	/* governor is only supported on Linux, so don't run this specific unit test on FreeBSD */

	CU_ADD_TEST(suite, test_governor);

#endif



	num_failures = spdk_ut_run_tests(argc, argv, NULL);

	CU_cleanup_registry();