Commit 16e5e505 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Jim Harris
Browse files

bdev: Add spdk_bdev_open_ext_v2() to support per-open options 



We will add the DIF insert/strip feature into the generic bdev layer.
We want to make the feature option per bdev open.

There exists spdk_bdev_open_async_opts. However it is only for
spdk_bdev_open_async(). spdk_bdev_open_async() is not generally usable.

spdk_bdev_open_ext() does not receive any option structure via
parameter.

It is not practical to change the existing spdk_bdev_open_ext().

Hence, add a new API spdk_bdev_open_ext_v2() with spdk_bdev_open_opts
structure. We find many examples to use v2 in DPDK.

Add hide_metadata option as the first option of the spdk_bdev_open_opts
structure.

Last 7 bytes in spdk_bdev_open_opts structure are unused and are not
initialized by the caller. To zero out these clearly with minimal user
effort, add spdk_bdev_open_opts_init() for initialization.

opts in spdk_bdev_desc is a hot data. Put it into the first cache line
in spdk_bdev_desc.

Furthermore, add simple unit test for verification.

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


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <jim.harris@nvidia.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Community-CI: Community CI Samsung <spdk.community.ci.samsung@gmail.com>
parent 20b34660

include/spdk/bdev.h

+41 −0
Original line number Diff line number Diff line
@@ -444,6 +444,28 @@ struct spdk_bdev *spdk_bdev_first_leaf(void); 
 */

struct spdk_bdev *spdk_bdev_next_leaf(struct spdk_bdev *prev);



/**

 * Structure with optional synchronous bdev open parameters.

 */

struct spdk_bdev_open_opts {

	/* Size of this structure in bytes. */

	size_t size;



	/* To indicate that the upper layer do not want to use metadata

	 * with this bdev.

	 */

	bool hide_metadata;

};

SPDK_STATIC_ASSERT(sizeof(struct spdk_bdev_open_opts) == 16, "Incorrect size");



/**

 * Initialize bdev open options.

 *

 * \param opts Bdev open options.

 * \param opts_size Must be set to sizeof(struct spdk_bdev_open_opts).

 */

void spdk_bdev_open_opts_init(struct spdk_bdev_open_opts *opts, size_t opts_size);



/**

 * Open a block device for I/O operations.

 *
@@ -461,6 +483,25 @@ struct spdk_bdev *spdk_bdev_next_leaf(struct spdk_bdev *prev); 
int spdk_bdev_open_ext(const char *bdev_name, bool write, spdk_bdev_event_cb_t event_cb,

		       void *event_ctx, struct spdk_bdev_desc **desc);



/**

 * Open a block device for I/O operations with options.

 *

 * \param bdev_name Block device name to open.

 * \param write true is read/write access requested, false if read-only

 * \param event_cb notification callback to be called when the bdev triggers

 * asynchronous event such as bdev removal. This will always be called on the

 * same thread that spdk_bdev_open_ext() was called on. In case of removal event

 * the descriptor will have to be manually closed to make the bdev unregister

 * proceed.

 * \param event_ctx param for event_cb.

 * \param opts Option for block device open. If NULL, default values are used.

 * \param desc output parameter for the descriptor when operation is successful

 * \return 0 if operation is successful, suitable errno value otherwise

 */

int spdk_bdev_open_ext_v2(const char *bdev_name, bool write, spdk_bdev_event_cb_t event_cb,

			  void *event_ctx, struct spdk_bdev_open_opts *opts,

			  struct spdk_bdev_desc **desc);



/**

 * Block device asynchronous open callback.

 *

lib/bdev/bdev.c

+99 −10
Original line number Diff line number Diff line
@@ -336,6 +336,7 @@ struct spdk_bdev_desc { 
	bool				write;

	bool				memory_domains_supported;

	bool				accel_sequence_supported[SPDK_BDEV_NUM_IO_TYPES];

	struct spdk_bdev_open_opts	opts;

	struct spdk_thread		*thread;

	struct {

		spdk_bdev_event_cb_t event_fn;
@@ -8209,19 +8210,90 @@ bdev_open(struct spdk_bdev *bdev, bool write, struct spdk_bdev_desc *desc) 
	return 0;

}



static void

bdev_open_opts_get_defaults(struct spdk_bdev_open_opts *opts, size_t opts_size)

{

	if (!opts) {

		SPDK_ERRLOG("opts should not be NULL.\n");

		return;

	}



	if (!opts_size) {

		SPDK_ERRLOG("opts_size should not be zero.\n");

		return;

	}



	memset(opts, 0, opts_size);

	opts->size = opts_size;



#define FIELD_OK(field) \

	offsetof(struct spdk_bdev_open_opts, field) + sizeof(opts->field) <= opts_size



#define SET_FIELD(field, value) \

	if (FIELD_OK(field)) { \

		opts->field = value; \

	} \



	SET_FIELD(hide_metadata, false);



#undef FIELD_OK

#undef SET_FIELD

}



static void

bdev_open_opts_copy(struct spdk_bdev_open_opts *opts,

		    const struct spdk_bdev_open_opts *opts_src, size_t opts_size)

{

	assert(opts);

	assert(opts_src);



#define SET_FIELD(field) \

	if (offsetof(struct spdk_bdev_open_opts, field) + sizeof(opts->field) <= opts_size) { \

		opts->field = opts_src->field; \

	} \



	SET_FIELD(hide_metadata);



	opts->size = opts_src->size;



	/* We should not remove this statement, but need to update the assert statement

	 * if we add a new field, and also add a corresponding SET_FIELD statement.

	 */

	SPDK_STATIC_ASSERT(sizeof(struct spdk_bdev_open_opts) == 16, "Incorrect size");



#undef SET_FIELD

}



void

spdk_bdev_open_opts_init(struct spdk_bdev_open_opts *opts, size_t opts_size)

{

	struct spdk_bdev_open_opts opts_local;



	bdev_open_opts_get_defaults(&opts_local, sizeof(opts_local));

	bdev_open_opts_copy(opts, &opts_local, opts_size);

}



static int

bdev_desc_alloc(struct spdk_bdev *bdev, spdk_bdev_event_cb_t event_cb, void *event_ctx,

		struct spdk_bdev_desc **_desc)

		struct spdk_bdev_open_opts *user_opts, struct spdk_bdev_desc **_desc)

{

	struct spdk_bdev_desc *desc;

	struct spdk_bdev_open_opts opts;

	unsigned int i;



	bdev_open_opts_get_defaults(&opts, sizeof(opts));

	if (user_opts != NULL) {

		bdev_open_opts_copy(&opts, user_opts, user_opts->size);

	}



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

	if (desc == NULL) {

		SPDK_ERRLOG("Failed to allocate memory for bdev descriptor\n");

		return -ENOMEM;

	}



	desc->opts = opts;



	TAILQ_INIT(&desc->pending_media_events);

	TAILQ_INIT(&desc->free_media_events);
@@ -8230,6 +8302,14 @@ bdev_desc_alloc(struct spdk_bdev *bdev, spdk_bdev_event_cb_t event_cb, void *eve 
	desc->callback.ctx = event_ctx;

	spdk_spin_init(&desc->spinlock);



	if (desc->opts.hide_metadata) {

		if (spdk_bdev_is_md_separate(bdev)) {

			SPDK_ERRLOG("hide_metadata option is not supported with separate metadata.\n");

			bdev_desc_free(desc);

			return -EINVAL;

		}

	}



	if (bdev->media_events) {

		desc->media_events_buffer = calloc(MEDIA_EVENT_POOL_SIZE,

						   sizeof(*desc->media_events_buffer));
@@ -8260,7 +8340,8 @@ bdev_desc_alloc(struct spdk_bdev *bdev, spdk_bdev_event_cb_t event_cb, void *eve 


static int

bdev_open_ext(const char *bdev_name, bool write, spdk_bdev_event_cb_t event_cb,

	      void *event_ctx, struct spdk_bdev_desc **_desc)

	      void *event_ctx, struct spdk_bdev_open_opts *opts,

	      struct spdk_bdev_desc **_desc)

{

	struct spdk_bdev_desc *desc;

	struct spdk_bdev *bdev;
@@ -8273,7 +8354,7 @@ bdev_open_ext(const char *bdev_name, bool write, spdk_bdev_event_cb_t event_cb, 
		return -ENODEV;

	}



	rc = bdev_desc_alloc(bdev, event_cb, event_ctx, &desc);

	rc = bdev_desc_alloc(bdev, event_cb, event_ctx, opts, &desc);

	if (rc != 0) {

		return rc;

	}
@@ -8290,8 +8371,9 @@ bdev_open_ext(const char *bdev_name, bool write, spdk_bdev_event_cb_t event_cb, 
}



int

spdk_bdev_open_ext(const char *bdev_name, bool write, spdk_bdev_event_cb_t event_cb,

		   void *event_ctx, struct spdk_bdev_desc **_desc)

spdk_bdev_open_ext_v2(const char *bdev_name, bool write, spdk_bdev_event_cb_t event_cb,

		      void *event_ctx, struct spdk_bdev_open_opts *opts,

		      struct spdk_bdev_desc **_desc)

{

	int rc;
@@ -8301,12 +8383,19 @@ spdk_bdev_open_ext(const char *bdev_name, bool write, spdk_bdev_event_cb_t event 
	}



	spdk_spin_lock(&g_bdev_mgr.spinlock);

	rc = bdev_open_ext(bdev_name, write, event_cb, event_ctx, _desc);

	rc = bdev_open_ext(bdev_name, write, event_cb, event_ctx, opts, _desc);

	spdk_spin_unlock(&g_bdev_mgr.spinlock);



	return rc;

}



int

spdk_bdev_open_ext(const char *bdev_name, bool write, spdk_bdev_event_cb_t event_cb,

		   void *event_ctx, struct spdk_bdev_desc **_desc)

{

	return spdk_bdev_open_ext_v2(bdev_name, write, event_cb, event_ctx, NULL, _desc);

}



struct spdk_bdev_open_async_ctx {

	char					*bdev_name;

	spdk_bdev_event_cb_t			event_cb;
@@ -8381,7 +8470,7 @@ _bdev_open_async(struct spdk_bdev_open_async_ctx *ctx) 
	}



	ctx->rc = bdev_open_ext(ctx->bdev_name, ctx->write, ctx->event_cb, ctx->event_ctx,

				&ctx->desc);

				NULL, &ctx->desc);

	if (ctx->rc == 0 || ctx->opts.timeout_ms == 0) {

		goto exit;

	}
@@ -8635,7 +8724,7 @@ spdk_bdev_register(struct spdk_bdev *bdev) 
	}



	/* A descriptor is opened to prevent bdev deletion during examination */

	rc = bdev_desc_alloc(bdev, _tmp_bdev_event_cb, NULL, &desc);

	rc = bdev_desc_alloc(bdev, _tmp_bdev_event_cb, NULL, NULL, &desc);

	if (rc != 0) {

		spdk_bdev_unregister(bdev, NULL, NULL);

		return rc;
@@ -9089,7 +9178,7 @@ spdk_for_each_bdev(void *ctx, spdk_for_each_bdev_fn fn) 
	spdk_spin_lock(&g_bdev_mgr.spinlock);

	bdev = spdk_bdev_first();

	while (bdev != NULL) {

		rc = bdev_desc_alloc(bdev, _tmp_bdev_event_cb, NULL, &desc);

		rc = bdev_desc_alloc(bdev, _tmp_bdev_event_cb, NULL, NULL, &desc);

		if (rc != 0) {

			break;

		}
@@ -9133,7 +9222,7 @@ spdk_for_each_bdev_leaf(void *ctx, spdk_for_each_bdev_fn fn) 
	spdk_spin_lock(&g_bdev_mgr.spinlock);

	bdev = spdk_bdev_first_leaf();

	while (bdev != NULL) {

		rc = bdev_desc_alloc(bdev, _tmp_bdev_event_cb, NULL, &desc);

		rc = bdev_desc_alloc(bdev, _tmp_bdev_event_cb, NULL, NULL, &desc);

		if (rc != 0) {

			break;

		}

lib/bdev/spdk_bdev.map

+2 −0
Original line number Diff line number Diff line
@@ -18,6 +18,8 @@ 
	spdk_for_each_bdev;

	spdk_for_each_bdev_leaf;

	spdk_bdev_open_ext;

	spdk_bdev_open_ext_v2;

	spdk_bdev_open_opts_init;

	spdk_bdev_open_async;

	spdk_bdev_close;

	spdk_bdev_get_numa_id;

test/unit/lib/bdev/bdev.c/bdev_ut.c

+32 −0
Original line number Diff line number Diff line
@@ -7709,6 +7709,37 @@ get_device_stat_with_reset(void) 
	ut_fini_bdev();

}



static void

open_ext_v2_test(void)

{

	struct spdk_bdev_open_opts opts;

	struct spdk_bdev *bdev;

	struct spdk_bdev_desc *desc;

	int rc;



	bdev = allocate_bdev("bdev0");



	rc = spdk_bdev_open_ext_v2("bdev0", true, bdev_ut_event_cb, NULL, NULL, &desc);

	CU_ASSERT(rc == 0);

	SPDK_CU_ASSERT_FATAL(desc != NULL);

	CU_ASSERT(desc->write == true);

	CU_ASSERT(desc->opts.hide_metadata == false);



	spdk_bdev_close(desc);



	spdk_bdev_open_opts_init(&opts, sizeof(opts));

	opts.hide_metadata = true;



	rc = spdk_bdev_open_ext_v2("bdev0", true, bdev_ut_event_cb, NULL, &opts, &desc);

	CU_ASSERT(rc == 0);

	CU_ASSERT(desc->write == true);

	CU_ASSERT(desc->opts.hide_metadata == true);



	spdk_bdev_close(desc);



	free_bdev(bdev);

}



int

main(int argc, char **argv)

{
@@ -7781,6 +7812,7 @@ main(int argc, char **argv) 
	CU_ADD_TEST(suite, examine_claimed_manual);

	CU_ADD_TEST(suite, get_numa_id);

	CU_ADD_TEST(suite, get_device_stat_with_reset);

	CU_ADD_TEST(suite, open_ext_v2_test);



	allocate_cores(1);

	allocate_threads(1);