From 3044bd27d03bd57280500fbde029b0430aedca0c Mon Sep 17 00:00:00 2001 From: paul luse Date: Fri, 24 Apr 2020 17:11:03 -0400 Subject: [PATCH] lib/accel: remove RPC for setting the module This was added before the usage of having a SW engine and 2 HW engines was fully thought out. The current rules are: * if no HW engine specific enable RPC is sent, use SW * if a HW engine specific enable RPC is sent, use it * If a 2nd HW engine specific enable RPC is sent, ignore In this scheme there's no need for an RPC that lets the user choose which engine to use because they already do so when they enable an engine. Signed-off-by: paul luse Change-Id: I006ffb3b417f1e93bb061b29535d157ba66f03b4 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2033 Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- include/spdk_internal/accel_engine.h | 10 ---- lib/accel/Makefile | 2 +- lib/accel/accel_engine.c | 43 +++----------- lib/accel/accel_engine_rpc.c | 85 --------------------------- mk/spdk.lib_deps.mk | 2 +- module/event/subsystems/accel/accel.c | 8 +-- scripts/rpc.py | 10 ---- scripts/rpc/__init__.py | 1 - scripts/rpc/accel.py | 9 --- test/json_config/config_filter.py | 1 - 10 files changed, 10 insertions(+), 161 deletions(-) delete mode 100644 lib/accel/accel_engine_rpc.c delete mode 100644 scripts/rpc/accel.py diff --git a/include/spdk_internal/accel_engine.h b/include/spdk_internal/accel_engine.h index bb0b47bdc..ae4c38a6c 100644 --- a/include/spdk_internal/accel_engine.h +++ b/include/spdk_internal/accel_engine.h @@ -39,16 +39,6 @@ #include "spdk/accel_engine.h" #include "spdk/queue.h" -enum accel_module { - ACCEL_SW = 0, - ACCEL_AUTO = 1, - ACCEL_CBDMA = 2, - ACCEL_IDXD_DSA = 3, - ACCEL_MODULE_MAX = 4, -}; - -int accel_set_module(enum accel_module *opts); - struct spdk_accel_task { spdk_accel_completion_cb cb; uint8_t offload_ctx[0]; diff --git a/lib/accel/Makefile b/lib/accel/Makefile index 1226ddb7c..d288e4ea1 100644 --- a/lib/accel/Makefile +++ b/lib/accel/Makefile @@ -39,7 +39,7 @@ SO_MINOR := 1 SO_SUFFIX := $(SO_VER).$(SO_MINOR) LIBNAME = accel -C_SRCS = accel_engine.c accel_engine_rpc.c +C_SRCS = accel_engine.c SPDK_MAP_FILE = $(abspath $(CURDIR)/spdk_accel.map) diff --git a/lib/accel/accel_engine.c b/lib/accel/accel_engine.c index 79698a987..7257afe3b 100644 --- a/lib/accel/accel_engine.c +++ b/lib/accel/accel_engine.c @@ -37,7 +37,6 @@ #include "spdk/env.h" #include "spdk/event.h" -#include "spdk/log.h" #include "spdk/thread.h" #include "spdk/json.h" @@ -56,7 +55,6 @@ static struct spdk_accel_engine *g_sw_accel_engine = NULL; static struct spdk_accel_module_if *g_accel_engine_module = NULL; static spdk_accel_fini_cb g_fini_cb_fn = NULL; static void *g_fini_cb_arg = NULL; -enum accel_module g_active_accel_module = ACCEL_AUTO; /* Global list of registered accelerator modules */ static TAILQ_HEAD(, spdk_accel_module_if) spdk_accel_module_list = @@ -67,14 +65,6 @@ struct accel_io_channel { struct spdk_io_channel *ch; }; -int -accel_set_module(enum accel_module *opts) -{ - g_active_accel_module = *opts; - - return 0; -} - /* Registration of hw modules (currently supports only 1 at a time) */ void spdk_accel_hw_engine_register(struct spdk_accel_engine *accel_engine) @@ -165,31 +155,15 @@ accel_engine_create_cb(void *io_device, void *ctx_buf) { struct accel_io_channel *accel_ch = ctx_buf; - /* If they specify CBDMA and its not available, fail */ - if (g_active_accel_module == ACCEL_CBDMA && g_hw_accel_engine == NULL) { - SPDK_ERRLOG("CBDMA acceleration engine specified but not available.\n"); - return -EINVAL; - } - - if (g_active_accel_module == ACCEL_IDXD_DSA && g_hw_accel_engine == NULL) { - SPDK_ERRLOG("IDXD acceleration engine specified but not available.\n"); - return -EINVAL; - } - - /* For either HW or AUTO */ - if (g_active_accel_module > ACCEL_SW) { - if (g_hw_accel_engine != NULL) { - accel_ch->ch = g_hw_accel_engine->get_io_channel(); - if (accel_ch->ch != NULL) { - accel_ch->engine = g_hw_accel_engine; - return 0; - } + if (g_hw_accel_engine != NULL) { + accel_ch->ch = g_hw_accel_engine->get_io_channel(); + if (accel_ch->ch != NULL) { + accel_ch->engine = g_hw_accel_engine; + return 0; } } - /* Choose SW either because auto was selected and there was no HW, - * or because SW was selected. - */ + /* No hw engine enabled, use sw. */ accel_ch->ch = g_sw_accel_engine->get_io_channel(); assert(accel_ch->ch != NULL); accel_ch->engine = g_sw_accel_engine; @@ -249,12 +223,9 @@ spdk_accel_engine_module_finish_cb(void) void spdk_accel_write_config_json(struct spdk_json_write_ctx *w) { + /* TODO: call engine config_json entry points. */ spdk_json_write_array_begin(w); spdk_json_write_object_begin(w); - spdk_json_write_named_string(w, "method", "accel_set_module"); - spdk_json_write_named_object_begin(w, "params"); - spdk_json_write_named_uint32(w, "module", g_active_accel_module); - spdk_json_write_object_end(w); spdk_json_write_object_end(w); spdk_json_write_array_end(w); } diff --git a/lib/accel/accel_engine_rpc.c b/lib/accel/accel_engine_rpc.c deleted file mode 100644 index 7f050a437..000000000 --- a/lib/accel/accel_engine_rpc.c +++ /dev/null @@ -1,85 +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. - */ - -#include "spdk_internal/accel_engine.h" -#include "spdk/rpc.h" -#include "spdk/util.h" -#include "spdk/string.h" -#include "spdk_internal/log.h" - -struct rpc_accel_set_module { - enum accel_module module; -}; - -static const struct spdk_json_object_decoder rpc_accel_set_module_decoder[] = { - {"module", offsetof(struct rpc_accel_set_module, module), spdk_json_decode_uint32}, -}; - -static void -spdk_rpc_accel_set_module(struct spdk_jsonrpc_request *request, - const struct spdk_json_val *params) -{ - struct rpc_accel_set_module req = {}; - struct spdk_json_write_ctx *w; - int rc = 0; - - if (spdk_json_decode_object(params, rpc_accel_set_module_decoder, - SPDK_COUNTOF(rpc_accel_set_module_decoder), - &req)) { - SPDK_ERRLOG("spdk_json_decode_object failed\n"); - spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_PARSE_ERROR, - "spdk_json_decode_object failed"); - return; - } - - if (req.module >= ACCEL_MODULE_MAX) { - spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, - "Module value %d should be less than %d", req.module, - ACCEL_MODULE_MAX); - return; - } - - rc = accel_set_module(&req.module); - if (rc) { - spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, spdk_strerror(-rc)); - return; - } - - w = spdk_jsonrpc_begin_result(request); - if (w != NULL) { - spdk_json_write_bool(w, true); - spdk_jsonrpc_end_result(request, w); - } -} -SPDK_RPC_REGISTER("accel_set_module", spdk_rpc_accel_set_module, - SPDK_RPC_STARTUP) diff --git a/mk/spdk.lib_deps.mk b/mk/spdk.lib_deps.mk index 773771b47..2c24578be 100644 --- a/mk/spdk.lib_deps.mk +++ b/mk/spdk.lib_deps.mk @@ -57,7 +57,7 @@ DEPDIRS-reduce := log util DEPDIRS-thread := log util DEPDIRS-blob := log util thread -DEPDIRS-accel := thread log rpc thread util $(JSON_LIBS) +DEPDIRS-accel := log json thread DEPDIRS-jsonrpc := log util json DEPDIRS-virtio := log util json thread diff --git a/module/event/subsystems/accel/accel.c b/module/event/subsystems/accel/accel.c index a1819c001..e9ec538bf 100644 --- a/module/event/subsystems/accel/accel.c +++ b/module/event/subsystems/accel/accel.c @@ -60,18 +60,12 @@ spdk_accel_engine_subsystem_finish(void) spdk_accel_engine_finish(spdk_accel_engine_subsystem_finish_done, NULL); } -static void -spdk_accel_engine_subsystem_write_config_json(struct spdk_json_write_ctx *w) -{ - spdk_accel_write_config_json(w); -} - static struct spdk_subsystem g_spdk_subsystem_accel = { .name = "accel", .init = spdk_accel_engine_subsystem_initialize, .fini = spdk_accel_engine_subsystem_finish, .config = spdk_accel_engine_config_text, - .write_config_json = spdk_accel_engine_subsystem_write_config_json, + .write_config_json = NULL, }; SPDK_SUBSYSTEM_REGISTER(g_spdk_subsystem_accel); diff --git a/scripts/rpc.py b/scripts/rpc.py index 18fdf29b2..e2f577f8f 100755 --- a/scripts/rpc.py +++ b/scripts/rpc.py @@ -2133,16 +2133,6 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse p.add_argument('name', help='Name of the Open Channel bdev') p.set_defaults(func=bdev_ocssd_delete) - # accel framework - def accel_set_module(args): - rpc.accel.accel_set_module(args.client, - module=args.module) - - p = subparsers.add_parser('accel_set_module', - help='Set module option for the accelerator framework') - p.add_argument('-m', '--module', type=int, help='0 = auto-select, 1= Software, 2 = CBDMA') - p.set_defaults(func=accel_set_module) - # ioat def ioat_scan_accel_engine(args): pci_whitelist = [] diff --git a/scripts/rpc/__init__.py b/scripts/rpc/__init__.py index d5065d37a..6aafe1d09 100644 --- a/scripts/rpc/__init__.py +++ b/scripts/rpc/__init__.py @@ -1,7 +1,6 @@ import json import sys -from . import accel from . import app from . import bdev from . import blobfs diff --git a/scripts/rpc/accel.py b/scripts/rpc/accel.py deleted file mode 100644 index 78bab85d8..000000000 --- a/scripts/rpc/accel.py +++ /dev/null @@ -1,9 +0,0 @@ -def accel_set_module(client, module): - """Set the module for the acceleration framework. - - Args: - pmd: 0 = auto-select, 1 = Software, 2 = CBDMA - """ - params = {'module': module} - - return client.call('accel_set_module', params) diff --git a/test/json_config/config_filter.py b/test/json_config/config_filter.py index cd28dcbb5..4f81ab077 100755 --- a/test/json_config/config_filter.py +++ b/test/json_config/config_filter.py @@ -23,7 +23,6 @@ def sort_json_object(o): def filter_methods(do_remove_global_rpcs): global_rpcs = [ - 'accel_set_module', 'iscsi_set_options', 'nvmf_set_config', 'nvmf_set_max_subsystems',