From 815ce363a979a416917fec6acbb9d55960955291 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Wed, 9 Jun 2021 17:12:43 +0000 Subject: [PATCH] nvme: default use_cmb_sqs to false Using the CMB for SQs is not a standard use case. Performance can vary widely when using CMB for SQs and is typically not the configuration used for benchmarking. So let's change the default value here to 'false', users can still opt-in by setting this option to true in the spdk_nvme_ctrlr_opts structure prior to attach. Signed-off-by: Jim Harris Change-Id: Iab746ba777b04152ffb92fea2a2bb923a0a0bf21 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8227 Reviewed-by: Paul Luse Reviewed-by: Aleksey Marchuk Reviewed-by: Ben Walker Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins --- CHANGELOG.md | 5 +++++ lib/nvme/nvme_ctrlr.c | 2 +- test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c | 4 ++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49ef003ca..7d0d9cd7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,11 @@ Added a new function `spdk_nvme_ns_cmd_copy` to submit a Simple Copy Command to Update the spdk_nvme_generic_command_status_code structure with new status code according to the definition in nvme 1.4 spec. +spdk_nvme_ctrlr_get_default_ctrlr_opts now sets use_cmb_sqs to false. This means +that is a controller has a CMB and supports SQs in the CMB, SPDK will not use +the CMB for SQs by default - the user must set use_cmb_sqs to true in +the spdk_nvme_ctrlr_opts structure prior to controller attach. + ### rpc New RPC `bdev_rbd_register_cluster` and `bdev_rbd_unregister_cluster` was added, it allows to create diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 4f8175575..f9534175c 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -158,7 +158,7 @@ spdk_nvme_ctrlr_get_default_ctrlr_opts(struct spdk_nvme_ctrlr_opts *opts, size_t } \ SET_FIELD(num_io_queues, DEFAULT_MAX_IO_QUEUES); - SET_FIELD(use_cmb_sqs, true); + SET_FIELD(use_cmb_sqs, false); SET_FIELD(no_shn_notification, false); SET_FIELD(arb_mechanism, SPDK_NVME_CC_AMS_RR); SET_FIELD(arbitration_burst, 0); diff --git a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c index 41a2b92ca..75a69a2cb 100644 --- a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c +++ b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c @@ -1785,7 +1785,7 @@ test_ctrlr_get_default_ctrlr_opts(void) CU_ASSERT(sizeof(opts) > 8); spdk_nvme_ctrlr_get_default_ctrlr_opts(&opts, 8); CU_ASSERT_EQUAL(opts.num_io_queues, DEFAULT_MAX_IO_QUEUES); - CU_ASSERT_TRUE(opts.use_cmb_sqs); + CU_ASSERT_FALSE(opts.use_cmb_sqs); /* check below fields are not initialized by default value */ CU_ASSERT_EQUAL(opts.arb_mechanism, 0); CU_ASSERT_EQUAL(opts.keep_alive_timeout_ms, 0); @@ -1805,7 +1805,7 @@ test_ctrlr_get_default_ctrlr_opts(void) /* set a consistent opts_size */ spdk_nvme_ctrlr_get_default_ctrlr_opts(&opts, sizeof(opts)); CU_ASSERT_EQUAL(opts.num_io_queues, DEFAULT_MAX_IO_QUEUES); - CU_ASSERT_TRUE(opts.use_cmb_sqs); + CU_ASSERT_FALSE(opts.use_cmb_sqs); CU_ASSERT_EQUAL(opts.arb_mechanism, SPDK_NVME_CC_AMS_RR); CU_ASSERT_EQUAL(opts.keep_alive_timeout_ms, 10 * 1000); CU_ASSERT_EQUAL(opts.io_queue_size, DEFAULT_IO_QUEUE_SIZE);