Commit 5322f307 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

lib/nvme: Fix aborting queued abort requests at controller reset or destruct 



ctrlr->outstanding_aborts is counted only for submitted abort requests.
However ctrlr->outstanding_aborts had been decremented for queued
abort requests by mistake.

Subsequent patches will use parent-children for abort requests but
nvme_free_request() is not aware of such relationship.

Queued abort requests had not been canceled or aborted when controller
was destructed. Retry submitting queued abort requests had been
repeated recursively and had caused stack overflow.

This patch fixes all.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I8ce0ae51ddd5ed3e1e8ac86329c8bdb7a9236b2f
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2555


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: default avatarMichael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 5cbc1d5c

lib/nvme/nvme_ctrlr.c

+21 −7
Original line number Diff line number Diff line
@@ -1156,12 +1156,28 @@ error: 
	return rc;

}



static void

nvme_ctrlr_abort_queued_aborts(struct spdk_nvme_ctrlr *ctrlr)

{

	struct nvme_request	*req, *tmp;

	struct spdk_nvme_cpl	cpl = {};



	cpl.status.sc = SPDK_NVME_SC_ABORTED_SQ_DELETION;

	cpl.status.sct = SPDK_NVME_SCT_GENERIC;



	STAILQ_FOREACH_SAFE(req, &ctrlr->queued_aborts, stailq, tmp) {

		STAILQ_REMOVE_HEAD(&ctrlr->queued_aborts, stailq);



		nvme_complete_request(req->cb_fn, req->cb_arg, req->qpair, req, &cpl);

		nvme_free_request(req);

	}

}



int

spdk_nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr)

{

	int rc = 0;

	struct spdk_nvme_qpair	*qpair;

	struct nvme_request	*req, *tmp;



	nvme_robust_mutex_lock(&ctrlr->ctrlr_lock);
@@ -1180,12 +1196,8 @@ spdk_nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr) 


	SPDK_NOTICELOG("resetting controller\n");



	/* Free all of the queued abort requests */

	STAILQ_FOREACH_SAFE(req, &ctrlr->queued_aborts, stailq, tmp) {

		STAILQ_REMOVE_HEAD(&ctrlr->queued_aborts, stailq);

		nvme_free_request(req);

		ctrlr->outstanding_aborts--;

	}

	/* Abort all of the queued abort requests */

	nvme_ctrlr_abort_queued_aborts(ctrlr);



	nvme_transport_admin_qpair_abort_aers(ctrlr->adminq);
@@ -2832,6 +2844,8 @@ nvme_ctrlr_destruct(struct spdk_nvme_ctrlr *ctrlr) 
	ctrlr->is_destructed = true;



	spdk_nvme_qpair_process_completions(ctrlr->adminq, 0);



	nvme_ctrlr_abort_queued_aborts(ctrlr);

	nvme_transport_admin_qpair_abort_aers(ctrlr->adminq);



	TAILQ_FOREACH_SAFE(qpair, &ctrlr->active_io_qpairs, tailq, tmp) {

lib/nvme/nvme_ctrlr_cmd.c

+4 −0
Original line number Diff line number Diff line
@@ -570,6 +570,10 @@ nvme_ctrlr_retry_queued_abort(struct spdk_nvme_ctrlr *ctrlr) 
	struct nvme_request	*next, *tmp;

	int rc;



	if (ctrlr->is_resetting || ctrlr->is_destructed) {

		return;

	}



	STAILQ_FOREACH_SAFE(next, &ctrlr->queued_aborts, stailq, tmp) {

		STAILQ_REMOVE_HEAD(&ctrlr->queued_aborts, stailq);

		ctrlr->outstanding_aborts++;