Commit d3d8b400 authored by Tomasz Zawadzki's avatar Tomasz Zawadzki Committed by Tomasz Zawadzki
Browse files

lib/nvme: use changed attached namespace list 



Previously the buffer from the log page went unused, while we could limit the
number of identified namespaces to just that list.

The log page contains namespaces which were just made active or were resized,
those are exactly the ones we want to identify.
Inactive namespaces do not need to identified and are already removed from
the list of active namespaces after identify to the controller was sent,
skip those.

Change-Id: I406ed3969d560b79c0ed848815d99b5d692ca37d
Signed-off-by: default avatarTomasz Zawadzki <tomasz.zawadzki@nutanix.com>
Reviewed-on: https://review.spdk.io/c/spdk/spdk/+/26305


Reviewed-by: default avatarJim Harris <jim.harris@nvidia.com>
Reviewed-by: default avatarJacek Kalwas <jacek.kalwas@nutanix.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK Automated Test System <spdkbot@gmail.com>
Reviewed-by: default avatarBen Walker <ben@nvidia.com>
parent efcfc4d6

lib/nvme/nvme_ctrlr.c

+28 −3
Original line number Diff line number Diff line
@@ -3205,15 +3205,40 @@ static void 
nvme_ctrlr_update_namespaces(struct spdk_nvme_ctrlr_aer_completion *async_event)

{

	struct spdk_nvme_ctrlr *ctrlr = async_event->ctrlr;

	uint32_t nsid;

	uint32_t nsid, i;

	struct spdk_nvme_ns *ns;



	/* Log page is not used, go over all active namespaces.

	 * Either the log page overflowed or disable_read_changed_ns_list_log_page is used. */

	if (async_event->log_page.changed_ns_list == NULL) {

		for (nsid = spdk_nvme_ctrlr_get_first_active_ns(ctrlr);

		     nsid != 0; nsid = spdk_nvme_ctrlr_get_next_active_ns(ctrlr, nsid)) {

			ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid);

			nvme_ns_construct(ns, nsid, ctrlr);

		}



		return;

	}



	/* Iterate over NSID from the log page. */

	for (i = 0; i < SPDK_NVME_MAX_CHANGED_NAMESPACES; i++) {

		nsid = async_event->log_page.changed_ns_list[i];



		/* End of the list */

		if (nsid == 0) {

			break;

		}



		/* Log page contains NSID for namespaces that were marked

		 * as inactive, no need to identify them. */

		if (!spdk_nvme_ctrlr_is_active_ns(ctrlr, nsid)) {

			continue;

		}



		ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid);

		nvme_ns_construct(ns, nsid, ctrlr);

	}



	spdk_dma_free(async_event->log_page.changed_ns_list);

}

test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c

+15 −6
Original line number Diff line number Diff line
@@ -3061,7 +3061,8 @@ test_nvme_ctrlr_ns_attr_changed(void) 
				changed_ns_list2, SPDK_COUNTOF(changed_ns_list2));

	nvme_ctrlr_complete_queued_async_events(&ctrlr);

	CU_ASSERT(g_aer_cb_counter == 1);

	CU_ASSERT(g_nvme_ns_constructed == g_active_ns_list_length);

	/* No NS constructed, NS 100 removal done by identify of active NS. */

	CU_ASSERT(g_nvme_ns_constructed == 0);

	check_active_ns(&ctrlr, active_ns_list2, SPDK_COUNTOF(active_ns_list2));

	CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 100));
@@ -3072,7 +3073,8 @@ test_nvme_ctrlr_ns_attr_changed(void) 
				changed_ns_list3, SPDK_COUNTOF(changed_ns_list3));

	nvme_ctrlr_complete_queued_async_events(&ctrlr);

	CU_ASSERT(g_aer_cb_counter == 1);

	CU_ASSERT(g_nvme_ns_constructed == g_active_ns_list_length);

	/* NS 101 constructed. */

	CU_ASSERT(g_nvme_ns_constructed == 1);

	check_active_ns(&ctrlr, active_ns_list3, SPDK_COUNTOF(active_ns_list3));



	/* Add NS 102, remove NS 101 */
@@ -3082,7 +3084,8 @@ test_nvme_ctrlr_ns_attr_changed(void) 
				changed_ns_list4, SPDK_COUNTOF(changed_ns_list4));

	nvme_ctrlr_complete_queued_async_events(&ctrlr);

	CU_ASSERT(g_aer_cb_counter == 1);

	CU_ASSERT(g_nvme_ns_constructed == g_active_ns_list_length);

	/* NS 102 constructed, NS 101 removal done by identify of active NS. */

	CU_ASSERT(g_nvme_ns_constructed == 1);

	check_active_ns(&ctrlr, active_ns_list4, SPDK_COUNTOF(active_ns_list4));

	CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 101));
@@ -3107,7 +3110,8 @@ test_nvme_ctrlr_ns_attr_changed(void) 
	 * Note: Identify sent to the controller after reading the log

	 * page will already get the current list of namespaces.

	 */

	CU_ASSERT(g_nvme_ns_constructed == (SPDK_COUNTOF(active_ns_list5_aer2) * 2));

	/* NS 103 constructed, NS 102 removal done by identify of active NS. */

	CU_ASSERT(g_nvme_ns_constructed == 1);

	check_active_ns(&ctrlr, active_ns_list5_aer2, SPDK_COUNTOF(active_ns_list5_aer2));

	CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 102));
@@ -3132,7 +3136,9 @@ test_nvme_ctrlr_ns_attr_changed(void) 
	 * Note: Identify sent to the controller after reading the log

	 * page will already get the current list of namespaces.

	 */

	CU_ASSERT(g_nvme_ns_constructed == (SPDK_COUNTOF(active_ns_list6_aer2) * 2));

	/* NS 105 constructed, NS 103 removal done by identify of active NS.

	 * NS 104 was never created, nor removed. */

	CU_ASSERT(g_nvme_ns_constructed == 1);

	check_active_ns(&ctrlr, active_ns_list6_aer2, SPDK_COUNTOF(active_ns_list6_aer2));

	CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 103));

	CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 104));
@@ -3172,7 +3178,10 @@ test_nvme_ctrlr_ns_attr_changed(void) 
				changed_ns_list8, SPDK_COUNTOF(changed_ns_list8));

	nvme_ctrlr_complete_queued_async_events(&ctrlr);

	CU_ASSERT(g_aer_cb_counter == 1);

	CU_ASSERT(g_nvme_ns_constructed == g_active_ns_list_length);

	/* NS 107, 109, 110 constructed.

	 * NS 105, 106 removal done by identify of active NS.

	 * NS 108 was never created, nor removed. */

	CU_ASSERT(g_nvme_ns_constructed == 3);

	check_active_ns(&ctrlr, active_ns_list8, SPDK_COUNTOF(active_ns_list8));

	CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 105));

	CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 106));