Commit f470a0dc authored by Jim Harris's avatar Jim Harris Committed by Tomasz Zawadzki
Browse files

event: do not call reactor events from spdk_thread context 



There are no remaining events that require running in an spdk_thread
context, so remove that code so that spdk_events are explicitly *not*
called from an spdk_thread context.

This is the desired behavior, there should be a clear delineation between
reactor context and spdk_thread context, and this change ensures that we
can send events that will not be run in an spdk_thread context.

So now spdk_reactor_set_interrupt_mode() checks that it was called on the
scheduling reactor, and not from the context of an spdk_thread.

This also requires changing the interrupt_tgt's reactor_set_interrupt_mode
RPC that is used for testing purposes. That RPC would just call
spdk_reactor_set_interrupt_mode() from its spdk_thread context, but
we need to use spdk_event_call() instead so that it gets called from
reactor context.

Signed-off-by: default avatarJim Harris <jim.harris@samsung.com>
Signed-off-by: default avatarsyeon.shin <syeon.shin@samsung.com>
Change-Id: Iadfae6f0943ad77076ffb8e81e5154d998e7a6f4
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/23261


Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 8d3fdcab

examples/interrupt_tgt/interrupt_tgt.c

+54 −20
Original line number Diff line number Diff line
@@ -11,12 +11,16 @@ 
#include "spdk/jsonrpc.h"

#include "spdk/rpc.h"

#include "spdk/env.h"

#include "spdk/scheduler.h"



#include "spdk_internal/event.h"



struct rpc_reactor_set_interrupt_mode {

	int32_t lcore;

	bool disable_interrupt;

	int rc;

	struct spdk_thread *rpc_thread;

	struct spdk_jsonrpc_request *request;

};



static const struct spdk_json_object_decoder rpc_reactor_set_interrupt_mode_decoders[] = {
@@ -27,26 +31,59 @@ static const struct spdk_json_object_decoder rpc_reactor_set_interrupt_mode_deco 
static void

rpc_reactor_set_interrupt_mode_cb(void *cb_arg)

{

	struct spdk_jsonrpc_request *request = cb_arg;

	struct rpc_reactor_set_interrupt_mode *req = cb_arg;



	SPDK_NOTICELOG("complete reactor switch\n");



	spdk_jsonrpc_send_bool_response(request, true);

	spdk_jsonrpc_send_bool_response(req->request, true);

	free(req);

}



static void

set_interrupt_mode_cb(void *arg)

{

	struct rpc_reactor_set_interrupt_mode *req = arg;



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

}



static void

set_interrupt_mode(void *arg1, void *arg2)

{

	struct rpc_reactor_set_interrupt_mode *req = arg1;

	int rc;



	rc = spdk_reactor_set_interrupt_mode(req->lcore, !req->disable_interrupt,

					     set_interrupt_mode_cb, req);

	if (rc)	{

		req->rc = rc;

		set_interrupt_mode_cb(req);

	}

}



static void

rpc_reactor_set_interrupt_mode(struct spdk_jsonrpc_request *request,

			       const struct spdk_json_val *params)

{

	struct rpc_reactor_set_interrupt_mode req = {};

	int rc;

	struct rpc_reactor_set_interrupt_mode *req;



	req = calloc(1, sizeof(*req));

	if (req == NULL) {

		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,

						 "Out of memory");

		return;

	}



	req->request = request;

	req->rpc_thread = spdk_get_thread();



	if (spdk_json_decode_object(params, rpc_reactor_set_interrupt_mode_decoders,

				    SPDK_COUNTOF(rpc_reactor_set_interrupt_mode_decoders),

				    &req)) {

				    req)) {

		SPDK_ERRLOG("spdk_json_decode_object failed\n");

		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,

						 "spdk_json_decode_object failed");

		free(req);

		return;

	}
@@ -54,29 +91,26 @@ rpc_reactor_set_interrupt_mode(struct spdk_jsonrpc_request *request, 
		SPDK_ERRLOG("Interrupt mode is not set when staring the application\n");

		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,

						 "spdk_json_decode_object failed");

		free(req);

		return;

	}





	SPDK_NOTICELOG("RPC Start to %s interrupt mode on reactor %d.\n",

		       req.disable_interrupt ? "disable" : "enable", req.lcore);

	if (req.lcore >= (int64_t)spdk_env_get_first_core() &&

	    req.lcore <= (int64_t)spdk_env_get_last_core()) {

		rc = spdk_reactor_set_interrupt_mode(req.lcore, !req.disable_interrupt,

						     rpc_reactor_set_interrupt_mode_cb, request);

		if (rc)	{

			goto err;

		}

		       req->disable_interrupt ? "disable" : "enable", req->lcore);

	if (req->lcore >= (int64_t)spdk_env_get_first_core() &&

	    req->lcore <= (int64_t)spdk_env_get_last_core()) {

		struct spdk_event *e;



		e = spdk_event_allocate(spdk_scheduler_get_scheduling_lcore(),

					set_interrupt_mode, req, NULL);

		spdk_event_call(e);

	} else {

		goto err;

	}



	return;



err:

		free(req);

		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,

						 "Invalid parameters");

	}

}

/* private */ SPDK_RPC_REGISTER("reactor_set_interrupt_mode", rpc_reactor_set_interrupt_mode,

				SPDK_RPC_RUNTIME)

lib/event/reactor.c

+3 −17
Original line number Diff line number Diff line
@@ -428,8 +428,8 @@ spdk_reactor_set_interrupt_mode(uint32_t lcore, bool new_in_interrupt, 
		return -ENOTSUP;

	}



	if (!spdk_thread_is_app_thread(NULL)) {

		SPDK_ERRLOG("It is only permitted within spdk application thread.\n");

	if (spdk_env_get_current_core() != g_scheduling_reactor->lcore) {

		SPDK_ERRLOG("It is only permitted within scheduling reactor.\n");

		return -EPERM;

	}
@@ -535,8 +535,6 @@ event_queue_run_batch(void *arg) 
	struct spdk_reactor *reactor = arg;

	size_t count, i;

	void *events[SPDK_EVENT_BATCH_SIZE];

	struct spdk_thread *thread;

	struct spdk_lw_thread *lw_thread;



#ifdef DEBUG

	/*
@@ -580,26 +578,14 @@ event_queue_run_batch(void *arg) 
		return 0;

	}



	/* Execute the events. There are still some remaining events

	 * that must occur on an SPDK thread. To accommodate those, try to

	 * run them on the first thread in the list, if it exists. */

	lw_thread = TAILQ_FIRST(&reactor->threads);

	if (lw_thread) {

		thread = spdk_thread_get_from_ctx(lw_thread);

	} else {

		thread = NULL;

	}



	for (i = 0; i < count; i++) {

		struct spdk_event *event = events[i];



		assert(event != NULL);

		spdk_set_thread(thread);



		assert(spdk_get_thread() == NULL);

		SPDK_DTRACE_PROBE3(event_exec, event->fn,

				   event->arg1, event->arg2);

		event->fn(event->arg1, event->arg2);

		spdk_set_thread(NULL);

	}



	spdk_mempool_put_bulk(g_spdk_event_mempool, events, count);

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

+7 −0
Original line number Diff line number Diff line
@@ -213,6 +213,8 @@ test_reschedule_thread(void) 
	CU_ASSERT(lw_thread->resched == false);

	CU_ASSERT(TAILQ_EMPTY(&reactor->threads));



	spdk_set_thread(NULL);



	reactor = spdk_reactor_get(0);

	CU_ASSERT(reactor != NULL);

	MOCK_SET(spdk_env_get_current_core, 0);
@@ -393,6 +395,7 @@ test_reactor_stats(void) 
	idle2 = spdk_poller_register(poller_run_idle, (void *)300, 0);

	CU_ASSERT(idle2 != NULL);



	spdk_set_thread(NULL);

	_reactor_run(reactor);



	spdk_set_thread(thread1);
@@ -464,6 +467,7 @@ test_reactor_stats(void) 
	busy1 = spdk_poller_register(poller_run_busy, (void *)100, 0);

	CU_ASSERT(busy1 != NULL);



	spdk_set_thread(NULL);

	_reactor_run(reactor);



	spdk_set_thread(thread1);
@@ -636,6 +640,7 @@ test_scheduler(void) 
	reactor = spdk_reactor_get(0);

	CU_ASSERT(reactor != NULL);

	MOCK_SET(spdk_env_get_current_core, 0);

	spdk_set_thread(NULL);

	event_queue_run_batch(reactor);



	reactor = spdk_reactor_get(0);
@@ -828,6 +833,8 @@ test_bind_thread(void) 
	}

	CU_ASSERT(spdk_get_ticks() == current_time);



	spdk_set_thread(NULL);



	/* Thread on core 2 should be scheduled to core 0 */

	reactor = spdk_reactor_get(0);

	CU_ASSERT(reactor != NULL);