Commit 5feab0dd authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Jim Harris
Browse files

bdev/nvme: Remove spdk_bdev_first/next() calls from apply_firmware RPC 



The NVMe bdev module has been refined since the bdev_nvme_apply_firmware
RPC was added. It is enough to open any one NVMe bdev even if one NVMe
controller has multiple namespaces. Hence, let's remove
spdk_bdev_first/next() calls from rpc_bdev_nvme_apply_firmware() rather
than replacing these by spdk_for_each_bdev(). Additionally, not only
spdk_bdev_close() but also spdk_put_io_channel() and
spdk_jsonrpc_send_*_response() should be done on the original thread.
Hence, redirect to the original thread always when a NVMe command is
returned, and rename firm_ctx->thread by firm_ctx->orig_thread for
clarification. orig_thread will be better than comment.

Signed-off-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Change-Id: I90b1986584b3926980fd265e4fded194eb5a2d00
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16541


Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent ee1f1248

module/bdev/nvme/bdev_nvme_rpc.c

+49 −96
Original line number Diff line number Diff line
@@ -21,14 +21,6 @@ 
#include "spdk/log.h"

#include "spdk/bdev_module.h"



struct open_descriptors {

	void *desc;

	struct  spdk_bdev *bdev;

	TAILQ_ENTRY(open_descriptors) tqlst;

	struct spdk_thread *thread;

};

typedef TAILQ_HEAD(, open_descriptors) open_descriptors_t;



static int

rpc_decode_action_on_timeout(const struct spdk_json_val *val, void *out)

{
@@ -783,32 +775,21 @@ struct firmware_update_info { 
	unsigned int			size_remaining;

	unsigned int			offset;

	unsigned int			transfer;

	bool				success;



	void				*desc;

	struct spdk_bdev_desc		*desc;

	struct spdk_io_channel		*ch;

	struct spdk_thread		*orig_thread;

	struct spdk_jsonrpc_request	*request;

	struct spdk_nvme_ctrlr		*ctrlr;

	open_descriptors_t		desc_head;

	struct rpc_apply_firmware	req;

};



static void

_apply_firmware_cleanup(void *ctx)

apply_firmware_cleanup(struct firmware_update_info *firm_ctx)

{

	struct spdk_bdev_desc *desc = ctx;



	spdk_bdev_close(desc);

}



static void

apply_firmware_cleanup(void *cb_arg)

{

	struct open_descriptors			*opt, *tmp;

	struct firmware_update_info *firm_ctx = cb_arg;



	if (!firm_ctx) {

		return;

	}

	assert(firm_ctx != NULL);

	assert(firm_ctx->orig_thread == spdk_get_thread());



	if (firm_ctx->fw_image) {

		spdk_free(firm_ctx->fw_image);
@@ -820,28 +801,22 @@ apply_firmware_cleanup(void *cb_arg) 
		spdk_put_io_channel(firm_ctx->ch);

	}



	TAILQ_FOREACH_SAFE(opt, &firm_ctx->desc_head, tqlst, tmp) {

		TAILQ_REMOVE(&firm_ctx->desc_head, opt, tqlst);

		/* Close the underlying bdev on its same opened thread. */

		if (opt->thread && opt->thread != spdk_get_thread()) {

			spdk_thread_send_msg(opt->thread, _apply_firmware_cleanup, opt->desc);

		} else {

			spdk_bdev_close(opt->desc);

		}

		free(opt);

	if (firm_ctx->desc) {

		spdk_bdev_close(firm_ctx->desc);

	}



	free(firm_ctx);

}



static void

apply_firmware_complete_reset(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg)

_apply_firmware_complete_reset(void *ctx)

{

	struct spdk_json_write_ctx		*w;

	struct firmware_update_info *firm_ctx = cb_arg;

	struct firmware_update_info *firm_ctx = ctx;



	spdk_bdev_free_io(bdev_io);

	assert(firm_ctx->orig_thread == spdk_get_thread());



	if (!success) {

	if (!firm_ctx->success) {

		spdk_jsonrpc_send_error_response(firm_ctx->request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,

						 "firmware commit failed.");

		apply_firmware_cleanup(firm_ctx);
@@ -862,18 +837,32 @@ apply_firmware_complete_reset(struct spdk_bdev_io *bdev_io, bool success, void * 
}



static void

apply_firmware_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg)

apply_firmware_complete_reset(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg)

{

	struct firmware_update_info *firm_ctx = cb_arg;



	spdk_bdev_free_io(bdev_io);



	firm_ctx->success = success;



	spdk_thread_exec_msg(firm_ctx->orig_thread, _apply_firmware_complete_reset, firm_ctx);

}



static void apply_firmware_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg);



static void

_apply_firmware_complete(void *ctx)

{

	struct spdk_nvme_cmd			cmd = {};

	struct spdk_nvme_fw_commit		fw_commit;

	int					slot = 0;

	int					rc;

	struct firmware_update_info *firm_ctx = cb_arg;

	struct firmware_update_info *firm_ctx = ctx;

	enum spdk_nvme_fw_commit_action commit_action = SPDK_NVME_FW_COMMIT_REPLACE_AND_ENABLE_IMG;



	spdk_bdev_free_io(bdev_io);

	assert(firm_ctx->orig_thread == spdk_get_thread());



	if (!success) {

	if (!firm_ctx->success) {

		spdk_jsonrpc_send_error_response(firm_ctx->request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,

						 "firmware download failed .");

		apply_firmware_cleanup(firm_ctx);
@@ -920,6 +909,18 @@ apply_firmware_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg 
	}

}



static void

apply_firmware_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg)

{

	struct firmware_update_info *firm_ctx = cb_arg;



	spdk_bdev_free_io(bdev_io);



	firm_ctx->success = success;



	spdk_thread_exec_msg(firm_ctx->orig_thread, _apply_firmware_complete, firm_ctx);

}



static void

apply_firmware_open_cb(enum spdk_bdev_event_type type, struct spdk_bdev *bdev, void *event_ctx)

{
@@ -932,11 +933,7 @@ rpc_bdev_nvme_apply_firmware(struct spdk_jsonrpc_request *request, 
	int					rc;

	int					fd = -1;

	struct stat				fw_stat;

	struct spdk_nvme_ctrlr			*ctrlr;

	struct spdk_bdev			*bdev;

	struct spdk_bdev			*bdev2;

	struct open_descriptors			*opt;

	struct spdk_bdev_desc			*desc;

	struct spdk_nvme_cmd			cmd = {};

	struct firmware_update_info		*firm_ctx;
@@ -947,8 +944,8 @@ rpc_bdev_nvme_apply_firmware(struct spdk_jsonrpc_request *request, 
		return;

	}

	firm_ctx->fw_image = NULL;

	TAILQ_INIT(&firm_ctx->desc_head);

	firm_ctx->request = request;

	firm_ctx->orig_thread = spdk_get_thread();



	if (spdk_json_decode_object(params, rpc_apply_firmware_decoders,

				    SPDK_COUNTOF(rpc_apply_firmware_decoders), &firm_ctx->req)) {
@@ -957,65 +954,21 @@ rpc_bdev_nvme_apply_firmware(struct spdk_jsonrpc_request *request, 
		goto err;

	}



	if ((bdev = spdk_bdev_get_by_name(firm_ctx->req.bdev_name)) == NULL) {

	if (spdk_bdev_open_ext(firm_ctx->req.bdev_name, true, apply_firmware_open_cb, NULL,

			       &firm_ctx->desc) != 0) {

		spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,

						     "bdev %s were not found",

						     "bdev %s could not be opened",

						     firm_ctx->req.bdev_name);

		goto err;

	}

	bdev = spdk_bdev_desc_get_bdev(firm_ctx->desc);



	if ((ctrlr = bdev_nvme_get_ctrlr(bdev)) == NULL) {

	if ((firm_ctx->ctrlr = bdev_nvme_get_ctrlr(bdev)) == NULL) {

		spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,

						     "Controller information for %s were not found.",

						     firm_ctx->req.bdev_name);

		goto err;

	}

	firm_ctx->ctrlr = ctrlr;



	for (bdev2 = spdk_bdev_first(); bdev2; bdev2 = spdk_bdev_next(bdev2)) {



		if (bdev_nvme_get_ctrlr(bdev2) != ctrlr) {

			continue;

		}



		if (!(opt = malloc(sizeof(struct open_descriptors)))) {

			spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,

							 "Memory allocation error.");

			goto err;

		}



		if (spdk_bdev_open_ext(spdk_bdev_get_name(bdev2), true, apply_firmware_open_cb, NULL, &desc) != 0) {

			spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,

							     "Device %s is in use.",

							     firm_ctx->req.bdev_name);

			free(opt);

			goto err;

		}



		/* Save the thread where the base device is opened */

		opt->thread = spdk_get_thread();



		opt->desc = desc;

		opt->bdev = bdev;

		TAILQ_INSERT_TAIL(&firm_ctx->desc_head, opt, tqlst);

	}



	/*

	 * find a descriptor associated with our bdev

	 */

	firm_ctx->desc = NULL;

	TAILQ_FOREACH(opt, &firm_ctx->desc_head, tqlst) {

		if (opt->bdev == bdev) {

			firm_ctx->desc = opt->desc;

			break;

		}

	}



	if (!firm_ctx->desc) {

		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,

						 "No descriptor were found.");

		goto err;

	}



	firm_ctx->ch = spdk_bdev_get_io_channel(firm_ctx->desc);

	if (!firm_ctx->ch) {