Commit a119799b authored by Changpeng Liu's avatar Changpeng Liu Committed by Tomasz Zawadzki
Browse files

test/nvme/aer: remove duplicated changed NS list log 



The NVMe driver layer will clear this log, so we don't
need to send another one in the aer callback.

Here we change the logic to compare with previous NS
state, if the NS state is same it will fail the test.

Change-Id: I6d80cb6a5f6d5eab92b8ccac601a23c19cea4003
Signed-off-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8175


Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarZiye Yang <ziye.yang@intel.com>
parent 441431d2

lib/nvme/nvme_ctrlr.c

+8 −23
Original line number Diff line number Diff line
@@ -2677,19 +2677,20 @@ nvme_ctrlr_process_async_event(struct spdk_nvme_ctrlr *ctrlr, 
{

	union spdk_nvme_async_event_completion event;

	struct spdk_nvme_ctrlr_process *active_proc;

	bool				ns_changed = false;

	int rc;



	event.raw = cpl->cdw0;



	if ((event.bits.async_event_type == SPDK_NVME_ASYNC_EVENT_TYPE_NOTICE) &&

	    (event.bits.async_event_info == SPDK_NVME_ASYNC_EVENT_NS_ATTR_CHANGED)) {

		/*

		 * apps (e.g., test/nvme/aer/aer.c) may also get changed ns log (through

		 * active_proc->aer_cb_fn). To avoid impaction, move our operations

		 * behind call of active_proc->aer_cb_fn.

		 */

		ns_changed = true;

		nvme_ctrlr_clear_changed_ns_log(ctrlr);



		rc = nvme_ctrlr_identify_active_ns(ctrlr);

		if (rc) {

			return;

		}

		nvme_ctrlr_update_namespaces(ctrlr);

		nvme_io_msg_ctrlr_update(ctrlr);

	}



	if ((event.bits.async_event_type == SPDK_NVME_ASYNC_EVENT_TYPE_NOTICE) &&
@@ -2705,22 +2706,6 @@ nvme_ctrlr_process_async_event(struct spdk_nvme_ctrlr *ctrlr, 
	if (active_proc && active_proc->aer_cb_fn) {

		active_proc->aer_cb_fn(active_proc->aer_cb_arg, cpl);

	}



	if (ns_changed) {

		/*

		 * Must have the changed ns log cleared by getting it.

		 * Otherwise, the target won't send

		 * the subsequent ns enabling/disabling events to us.

		 */

		nvme_ctrlr_clear_changed_ns_log(ctrlr);



		rc = nvme_ctrlr_identify_active_ns(ctrlr);

		if (rc) {

			return;

		}

		nvme_ctrlr_update_namespaces(ctrlr);

		nvme_io_msg_ctrlr_update(ctrlr);

	}

}



static void

test/nvme/aer/aer.c

+12 −63
Original line number Diff line number Diff line
@@ -42,8 +42,9 @@ 


struct dev {

	struct spdk_nvme_ctrlr				*ctrlr;

	/* Expected changed NS ID state before AER */

	bool						ns_test_active;

	struct spdk_nvme_health_information_page	*health_page;

	struct spdk_nvme_ns_list			*changed_ns_list;

	uint32_t					orig_temp_threshold;

	char						name[SPDK_NVMF_TRADDR_MAX_LEN + 1];

};
@@ -65,9 +66,7 @@ static char *g_touch_file; 


/* Enable AER temperature test */

static int g_enable_temp_test = 0;

/* Enable AER namespace attribute notice test, this variable holds

 * the NSID that is expected to be in the Changed NS List.

 */

/* Expected changed NS ID */

static uint32_t g_expected_ns_test = 0;



static void
@@ -167,43 +166,6 @@ get_health_log_page_completion(void *cb_arg, const struct spdk_nvme_cpl *cpl) 
	g_aer_done++;

}



static void

get_changed_ns_log_page_completion(void *cb_arg, const struct spdk_nvme_cpl *cpl)

{

	struct dev *dev = cb_arg;

	bool found = false;

	uint32_t i;



	g_outstanding_commands --;



	if (spdk_nvme_cpl_is_error(cpl)) {

		printf("%s: get log page failed\n", dev->name);

		g_failed = 1;

		return;

	}



	/* Let's compare the expected namespce ID is

	 * in changed namespace list

	 */

	if (dev->changed_ns_list->ns_list[0] != 0xffffffffu) {

		for (i = 0; i < sizeof(*dev->changed_ns_list) / sizeof(uint32_t); i++) {

			if (g_expected_ns_test == dev->changed_ns_list->ns_list[i]) {

				printf("%s: changed NS list contains expected NSID: %u\n",

				       dev->name, g_expected_ns_test);

				found = true;

				break;

			}

		}

	}



	if (!found) {

		printf("%s: Error: Can't find expected NSID %u\n", dev->name, g_expected_ns_test);

		g_failed = 1;

	}



	g_aer_done++;

}



static int

get_health_log_page(struct dev *dev)

{
@@ -220,21 +182,15 @@ get_health_log_page(struct dev *dev) 
	return rc;

}



static int

get_changed_ns_log_page(struct dev *dev)

static void

get_ns_state_test(struct dev *dev, uint32_t nsid)

{

	int rc;



	rc = spdk_nvme_ctrlr_cmd_get_log_page(dev->ctrlr, SPDK_NVME_LOG_CHANGED_NS_LIST,

					      SPDK_NVME_GLOBAL_NS_TAG, dev->changed_ns_list,

					      sizeof(*dev->changed_ns_list), 0,

					      get_changed_ns_log_page_completion, dev);

	bool new_ns_state;



	if (rc == 0) {

		g_outstanding_commands++;

	new_ns_state = spdk_nvme_ctrlr_is_active_ns(dev->ctrlr, nsid);

	if (new_ns_state == dev->ns_test_active) {

		g_failed = 1;

	}



	return rc;

}



static void
@@ -246,9 +202,6 @@ cleanup(void) 
		if (dev->health_page) {

			spdk_free(dev->health_page);

		}

		if (dev->changed_ns_list) {

			spdk_free(dev->changed_ns_list);

		}

	}

}
@@ -273,7 +226,8 @@ aer_cb(void *arg, const struct spdk_nvme_cpl *cpl) 
		set_temp_threshold(dev, dev->orig_temp_threshold);

		get_health_log_page(dev);

	} else if (log_page_id == SPDK_NVME_LOG_CHANGED_NS_LIST) {

		get_changed_ns_log_page(dev);

		get_ns_state_test(dev, g_expected_ns_test);

		g_aer_done++;

	}

}
@@ -387,12 +341,6 @@ attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, 
		printf("Allocation error (health page)\n");

		g_failed = 1;

	}

	dev->changed_ns_list = spdk_zmalloc(sizeof(*dev->changed_ns_list), 4096, NULL,

					    SPDK_ENV_LCORE_ID_ANY, SPDK_MALLOC_DMA);

	if (dev->changed_ns_list == NULL) {

		printf("Allocation error (changed namespace list page)\n");

		g_failed = 1;

	}

}



static void
@@ -501,6 +449,7 @@ spdk_aer_changed_ns_test(void) 


	foreach_dev(dev) {

		get_feature_test(dev);

		dev->ns_test_active = spdk_nvme_ctrlr_is_active_ns(dev->ctrlr, g_expected_ns_test);

	}



	if (g_failed) {