From 4bcfef0021be2daf9b792f45b02e93f6c9c13eca Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Mon, 6 Nov 2017 09:31:15 -0700 Subject: [PATCH] rpc: remove rpc subsystem RPC is core functionality for SPDK applications and should always be initialized last (after all subsystems have been initialized). So make RPC a first class citizen and integrate it with the app framework directly instead of making it an "optional" subsystem. Then we initializing it after all subsystems have completed initialization, and tear it down before tearing down subsystems. We can also do some other cleanup while here - for example, reactors are already started when spdk_rpc_initialize() is called, so remove the extra event that was added during initialization since this is no longer needed. Signed-off-by: Jim Harris Change-Id: I4cc63586a6d55be68786629a2176c61a88979267 Reviewed-on: https://review.gerrithub.io/385914 Reviewed-by: Tomasz Zawadzki Reviewed-by: Daniel Verkamp Tested-by: SPDK Automated Test System Reviewed-by: Ben Walker --- app/iscsi_tgt/Makefile | 2 +- app/nvmf_tgt/Makefile | 2 +- app/vhost/Makefile | 2 +- include/spdk_internal/event.h | 4 +++ lib/event/Makefile | 2 +- lib/event/app.c | 19 ++++++++++++- lib/event/{subsystems/rpc => }/rpc.c | 34 ++++------------------- lib/event/subsystems/Makefile | 2 +- lib/event/subsystems/iscsi/iscsi.c | 1 - lib/event/subsystems/rpc/Makefile | 41 ---------------------------- lib/rocksdb/spdk.rocksdb.mk | 2 +- test/app/bdev_svc/Makefile | 2 +- test/app/stub/Makefile | 2 +- test/lib/bdev/bdevio/Makefile | 2 +- test/lib/bdev/bdevperf/Makefile | 2 +- test/lib/bdev/nbd/Makefile | 2 +- test/lib/blobfs/mkfs/Makefile | 2 +- test/lib/event/event_perf/Makefile | 2 +- test/lib/event/reactor/Makefile | 2 +- test/lib/event/reactor_perf/Makefile | 2 +- 20 files changed, 43 insertions(+), 86 deletions(-) rename lib/event/{subsystems/rpc => }/rpc.c (81%) delete mode 100644 lib/event/subsystems/rpc/Makefile diff --git a/app/iscsi_tgt/Makefile b/app/iscsi_tgt/Makefile index f34a6f847..49479cc60 100644 --- a/app/iscsi_tgt/Makefile +++ b/app/iscsi_tgt/Makefile @@ -46,7 +46,7 @@ CFLAGS += -I$(SPDK_ROOT_DIR)/lib C_SRCS := iscsi_tgt.c -SPDK_LIB_LIST = event_bdev event_copy event_iscsi event_net event_rpc event_scsi +SPDK_LIB_LIST = event_bdev event_copy event_iscsi event_net event_scsi SPDK_LIB_LIST += jsonrpc json rpc bdev_rpc bdev iscsi scsi net copy trace conf SPDK_LIB_LIST += util log log_rpc event app_rpc diff --git a/app/nvmf_tgt/Makefile b/app/nvmf_tgt/Makefile index 0fb803fad..4dc715d07 100644 --- a/app/nvmf_tgt/Makefile +++ b/app/nvmf_tgt/Makefile @@ -42,7 +42,7 @@ CFLAGS += $(ENV_CFLAGS) C_SRCS := conf.c nvmf_main.c nvmf_tgt.c nvmf_rpc.c -SPDK_LIB_LIST = event_bdev event_copy event_rpc +SPDK_LIB_LIST = event_bdev event_copy SPDK_LIB_LIST += nvmf event log trace conf util bdev copy rpc jsonrpc json SPDK_LIB_LIST += app_rpc log_rpc bdev_rpc diff --git a/app/vhost/Makefile b/app/vhost/Makefile index 455ca76cc..d5fe12a25 100644 --- a/app/vhost/Makefile +++ b/app/vhost/Makefile @@ -42,7 +42,7 @@ CFLAGS += $(ENV_CFLAGS) C_SRCS := vhost.c -SPDK_LIB_LIST = event_bdev event_copy event_net event_rpc event_scsi event_vhost +SPDK_LIB_LIST = event_bdev event_copy event_net event_scsi event_vhost SPDK_LIB_LIST += jsonrpc json rpc bdev_rpc bdev scsi net copy trace conf SPDK_LIB_LIST += util log log_rpc event app_rpc SPDK_LIB_LIST += vhost rte_vhost diff --git a/include/spdk_internal/event.h b/include/spdk_internal/event.h index 5d823e6d8..aee71d5d5 100644 --- a/include/spdk_internal/event.h +++ b/include/spdk_internal/event.h @@ -76,6 +76,10 @@ void spdk_subsystem_init_next(int rc); void spdk_subsystem_fini_next(void); void spdk_subsystem_config(FILE *fp); +void spdk_rpc_initialize(void); +void spdk_rpc_finish(void); +void spdk_rpc_config_text(FILE *fp); + /** * \brief Register a new subsystem */ diff --git a/lib/event/Makefile b/lib/event/Makefile index 6689067d2..34e975395 100644 --- a/lib/event/Makefile +++ b/lib/event/Makefile @@ -36,7 +36,7 @@ include $(SPDK_ROOT_DIR)/mk/spdk.common.mk CFLAGS += $(ENV_CFLAGS) LIBNAME = event -C_SRCS = app.c reactor.c subsystem.c +C_SRCS = app.c reactor.c rpc.c subsystem.c DIRS-y = rpc subsystems diff --git a/lib/event/app.c b/lib/event/app.c index 1d0d952ff..a8f71c26d 100644 --- a/lib/event/app.c +++ b/lib/event/app.c @@ -58,6 +58,10 @@ static struct spdk_app g_spdk_app; static struct spdk_event *g_shutdown_event = NULL; static int g_init_lcore; +static spdk_event_fn g_app_start_fn; +static void *g_app_start_arg1; +static void *g_app_start_arg2; + int spdk_app_get_shm_id(void) { @@ -129,6 +133,7 @@ spdk_app_get_running_config(char **config_str, char *name) setvbuf(fp, vbuf, _IOFBF, BUFSIZ); spdk_app_config_dump_global_section(fp); + spdk_rpc_config_text(fp); spdk_subsystem_config(fp); length = ftell(fp); @@ -246,6 +251,13 @@ spdk_app_setup_signal_handlers(struct spdk_app_opts *opts) return 0; } +static void +start_rpc(void *arg1, void *arg2) +{ + spdk_rpc_initialize(); + g_app_start_fn(g_app_start_arg1, g_app_start_arg2); +} + int spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn, void *arg1, void *arg2) @@ -378,7 +390,10 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn, g_spdk_app.rc = 0; g_init_lcore = spdk_env_get_current_core(); - app_start_event = spdk_event_allocate(g_init_lcore, start_fn, arg1, arg2); + g_app_start_fn = start_fn; + g_app_start_arg1 = arg1; + g_app_start_arg2 = arg2; + app_start_event = spdk_event_allocate(g_init_lcore, start_rpc, NULL, NULL); spdk_subsystem_init(app_start_event); @@ -402,6 +417,8 @@ _spdk_app_stop(void *arg1, void *arg2) { struct spdk_event *app_stop_event; + spdk_rpc_finish(); + app_stop_event = spdk_event_allocate(spdk_env_get_current_core(), spdk_reactors_stop, NULL, NULL); spdk_subsystem_fini(app_stop_event); } diff --git a/lib/event/subsystems/rpc/rpc.c b/lib/event/rpc.c similarity index 81% rename from lib/event/subsystems/rpc/rpc.c rename to lib/event/rpc.c index 112542dec..29bbb6c35 100644 --- a/lib/event/subsystems/rpc/rpc.c +++ b/lib/event/rpc.c @@ -83,15 +83,12 @@ spdk_rpc_subsystem_poll(void *arg) spdk_rpc_accept(); } -static void -spdk_rpc_subsystem_setup(void *arg) +void +spdk_rpc_initialize(void) { const char *listen_addr; int rc; - /* Unregister the poller */ - spdk_poller_unregister(&g_rpc_poller, NULL); - if (!enable_rpc()) { return; } @@ -113,30 +110,15 @@ spdk_rpc_subsystem_setup(void *arg) RPC_SELECT_INTERVAL); } -static void -spdk_rpc_subsystem_initialize(void) -{ - /* - * Defer setup of the RPC service until the reactor has started. This - * allows us to detect the RPC listen socket as a suitable proxy for determining - * when the SPDK application has finished initialization and ready for logins - * or RPC commands. - */ - spdk_poller_register(&g_rpc_poller, spdk_rpc_subsystem_setup, NULL, spdk_env_get_current_core(), 0); - - spdk_subsystem_init_next(0); -} - -static void -spdk_rpc_subsystem_finish(void *arg1, void *arg2) +void +spdk_rpc_finish(void) { spdk_rpc_close(); spdk_poller_unregister(&g_rpc_poller, NULL); - spdk_subsystem_fini_next(); } -static void -spdk_rpc_subsystem_config_text(FILE *fp) +void +spdk_rpc_config_text(FILE *fp) { fprintf(fp, "\n" @@ -151,7 +133,3 @@ spdk_rpc_subsystem_config_text(FILE *fp) " Listen %s\n", enable_rpc() ? "Yes" : "No", rpc_get_listen_addr()); } - -SPDK_SUBSYSTEM_REGISTER(spdk_rpc, spdk_rpc_subsystem_initialize, - spdk_rpc_subsystem_finish, - spdk_rpc_subsystem_config_text) diff --git a/lib/event/subsystems/Makefile b/lib/event/subsystems/Makefile index 964723adc..d47b2c31d 100644 --- a/lib/event/subsystems/Makefile +++ b/lib/event/subsystems/Makefile @@ -34,7 +34,7 @@ SPDK_ROOT_DIR := $(abspath $(CURDIR)/../../..) include $(SPDK_ROOT_DIR)/mk/spdk.common.mk -DIRS-y += bdev copy iscsi net rpc scsi vhost +DIRS-y += bdev copy iscsi net scsi vhost .PHONY: all clean $(DIRS-y) diff --git a/lib/event/subsystems/iscsi/iscsi.c b/lib/event/subsystems/iscsi/iscsi.c index f3f22ac38..10604136a 100644 --- a/lib/event/subsystems/iscsi/iscsi.c +++ b/lib/event/subsystems/iscsi/iscsi.c @@ -57,4 +57,3 @@ spdk_iscsi_subsystem_fini(void *arg1, void *arg2) SPDK_SUBSYSTEM_REGISTER(iscsi, spdk_iscsi_subsystem_init, spdk_iscsi_subsystem_fini, spdk_iscsi_config_text) SPDK_SUBSYSTEM_DEPEND(iscsi, scsi) -SPDK_SUBSYSTEM_DEPEND(iscsi, spdk_rpc) diff --git a/lib/event/subsystems/rpc/Makefile b/lib/event/subsystems/rpc/Makefile deleted file mode 100644 index c9d16f7f5..000000000 --- a/lib/event/subsystems/rpc/Makefile +++ /dev/null @@ -1,41 +0,0 @@ -# -# BSD LICENSE -# -# Copyright (c) Intel Corporation. -# All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions -# are met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above copyright -# notice, this list of conditions and the following disclaimer in -# the documentation and/or other materials provided with the -# distribution. -# * Neither the name of Intel Corporation nor the names of its -# contributors may be used to endorse or promote products derived -# from this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -# - -SPDK_ROOT_DIR := $(abspath $(CURDIR)/../../../..) -include $(SPDK_ROOT_DIR)/mk/spdk.common.mk - -CFLAGS += $(ENV_CFLAGS) -I. -C_SRCS = rpc.c -LIBNAME = event_rpc - -include $(SPDK_ROOT_DIR)/mk/spdk.lib.mk diff --git a/lib/rocksdb/spdk.rocksdb.mk b/lib/rocksdb/spdk.rocksdb.mk index b415b2b57..d133ddb4a 100644 --- a/lib/rocksdb/spdk.rocksdb.mk +++ b/lib/rocksdb/spdk.rocksdb.mk @@ -53,7 +53,7 @@ ifeq ($(CONFIG_ASAN),y) CXXFLAGS += -fno-sanitize=address endif -SPDK_LIB_LIST = event_bdev event_copy event_rpc +SPDK_LIB_LIST = event_bdev event_copy SPDK_LIB_LIST += blobfs bdev copy event util conf trace \ log jsonrpc json rpc diff --git a/test/app/bdev_svc/Makefile b/test/app/bdev_svc/Makefile index 26892657b..ea70b2756 100644 --- a/test/app/bdev_svc/Makefile +++ b/test/app/bdev_svc/Makefile @@ -42,7 +42,7 @@ CFLAGS += $(ENV_CFLAGS) C_SRCS := bdev_svc.c -SPDK_LIB_LIST = event_bdev event_copy event_rpc +SPDK_LIB_LIST = event_bdev event_copy SPDK_LIB_LIST += nvmf event log trace conf util bdev copy rpc jsonrpc json SPDK_LIB_LIST += app_rpc log_rpc bdev_rpc diff --git a/test/app/stub/Makefile b/test/app/stub/Makefile index 2703c1e88..e1c94ab12 100644 --- a/test/app/stub/Makefile +++ b/test/app/stub/Makefile @@ -41,7 +41,7 @@ CFLAGS += $(ENV_CFLAGS) C_SRCS := stub.c -SPDK_LIB_LIST = event conf nvme util log trace +SPDK_LIB_LIST = event conf nvme util log trace rpc jsonrpc json LIBS += $(SPDK_LIB_LINKER_ARGS) LIBS += $(ENV_LINKER_ARGS) diff --git a/test/lib/bdev/bdevio/Makefile b/test/lib/bdev/bdevio/Makefile index a5e358391..8d68bd9c2 100644 --- a/test/lib/bdev/bdevio/Makefile +++ b/test/lib/bdev/bdevio/Makefile @@ -42,7 +42,7 @@ C_SRCS := bdevio.c CFLAGS += -I. $(ENV_CFLAGS) -SPDK_LIB_LIST = event_bdev event_copy event_rpc +SPDK_LIB_LIST = event_bdev event_copy SPDK_LIB_LIST += bdev copy event trace log conf util rpc jsonrpc json LIBS += $(BLOCKDEV_MODULES_LINKER_ARGS) \ diff --git a/test/lib/bdev/bdevperf/Makefile b/test/lib/bdev/bdevperf/Makefile index 2845f6c91..3c4bb3af6 100644 --- a/test/lib/bdev/bdevperf/Makefile +++ b/test/lib/bdev/bdevperf/Makefile @@ -42,7 +42,7 @@ C_SRCS := bdevperf.c CFLAGS += -I. $(ENV_CFLAGS) -SPDK_LIB_LIST = event_bdev event_copy event_rpc +SPDK_LIB_LIST = event_bdev event_copy SPDK_LIB_LIST += bdev copy event trace log conf util rpc jsonrpc json LIBS += $(BLOCKDEV_MODULES_LINKER_ARGS) \ diff --git a/test/lib/bdev/nbd/Makefile b/test/lib/bdev/nbd/Makefile index 3ef08ddbf..dff27aeb3 100644 --- a/test/lib/bdev/nbd/Makefile +++ b/test/lib/bdev/nbd/Makefile @@ -42,7 +42,7 @@ C_SRCS := nbd.c CFLAGS += -I. $(ENV_CFLAGS) -SPDK_LIB_LIST = event_bdev event_copy event_rpc nbd +SPDK_LIB_LIST = event_bdev event_copy nbd SPDK_LIB_LIST += bdev bdev_rpc copy event trace log log_rpc conf util rpc jsonrpc json LIBS += $(BLOCKDEV_MODULES_LINKER_ARGS) \ diff --git a/test/lib/blobfs/mkfs/Makefile b/test/lib/blobfs/mkfs/Makefile index 66fbe382e..c724e1c8a 100644 --- a/test/lib/blobfs/mkfs/Makefile +++ b/test/lib/blobfs/mkfs/Makefile @@ -40,7 +40,7 @@ APP = mkfs C_SRCS := mkfs.c -SPDK_LIB_LIST = event_bdev event_copy event_rpc +SPDK_LIB_LIST = event_bdev event_copy SPDK_LIB_LIST += blobfs blob bdev blob_bdev copy event util conf trace \ log jsonrpc json rpc diff --git a/test/lib/event/event_perf/Makefile b/test/lib/event/event_perf/Makefile index c9e5ca812..8e405582f 100644 --- a/test/lib/event/event_perf/Makefile +++ b/test/lib/event/event_perf/Makefile @@ -39,7 +39,7 @@ CFLAGS += $(ENV_CFLAGS) APP = event_perf C_SRCS := event_perf.c -SPDK_LIB_LIST = event trace conf util log +SPDK_LIB_LIST = event trace conf util log rpc jsonrpc json LIBS += $(SPDK_LIB_LINKER_ARGS) $(ENV_LINKER_ARGS) diff --git a/test/lib/event/reactor/Makefile b/test/lib/event/reactor/Makefile index 1d9bb6734..f9fb73dc8 100644 --- a/test/lib/event/reactor/Makefile +++ b/test/lib/event/reactor/Makefile @@ -39,7 +39,7 @@ CFLAGS += $(ENV_CFLAGS) APP = reactor C_SRCS := reactor.c -SPDK_LIB_LIST = event trace conf util log +SPDK_LIB_LIST = event trace conf util log rpc jsonrpc json LIBS += $(SPDK_LIB_LINKER_ARGS) $(ENV_LINKER_ARGS) diff --git a/test/lib/event/reactor_perf/Makefile b/test/lib/event/reactor_perf/Makefile index 220c1ab43..68e3e8c91 100644 --- a/test/lib/event/reactor_perf/Makefile +++ b/test/lib/event/reactor_perf/Makefile @@ -39,7 +39,7 @@ CFLAGS += $(ENV_CFLAGS) APP = reactor_perf C_SRCS := reactor_perf.c -SPDK_LIB_LIST = event trace conf util log +SPDK_LIB_LIST = event trace conf util log rpc jsonrpc json LIBS += $(SPDK_LIB_LINKER_ARGS) $(ENV_LINKER_ARGS)