Commit 43f6d338 authored by Jim Harris's avatar Jim Harris Committed by Konrad Sztyber
Browse files

nvmf: remove use of STAILQ for last_wqe events 



I think the original intention here was for possible processing
of multiple events. But use of STAILQ would actually be unsafe without
a lock in that case, since a second event could be inserted while
the targeted thread is removing the first event.

So just attach the ctx directly to the qpair. Existing code that
destroys the qpair while this message is en route can still set that
ctx->qpair to NULL to ensure the message is handled safely once
received.

While here also add a check to catch unexpected case where
LAST_WQE_REACHED is received more than once for the same rqpair.

Signed-off-by: default avatarJim Harris <jim.harris@samsung.com>
Change-Id: I24b76d4b50d7438b8c41a715f6e3e025769fb50c
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/24985


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 9645421c

lib/nvmf/rdma.c

+20 −13
Original line number Diff line number Diff line
@@ -306,8 +306,6 @@ typedef void (*spdk_poller_destroy_cb)(void *ctx); 


struct spdk_nvmf_rdma_ibv_event_ctx {

	struct spdk_nvmf_rdma_qpair			*rqpair;

	/* Link to other ibv events associated with this qpair */

	STAILQ_ENTRY(spdk_nvmf_rdma_ibv_event_ctx)	link;

};



struct spdk_nvmf_rdma_qpair {
@@ -382,8 +380,8 @@ struct spdk_nvmf_rdma_qpair { 
	 */

	struct spdk_io_channel		*destruct_channel;



	/* List of ibv async events */

	STAILQ_HEAD(, spdk_nvmf_rdma_ibv_event_ctx)	ibv_events;

	/* ctx for async processing of last_wqe_reached event */

	struct spdk_nvmf_rdma_ibv_event_ctx	*last_wqe_reached_ctx;



	/* Lets us know that we have received the last_wqe event. */

	bool					last_wqe_reached;
@@ -836,11 +834,12 @@ cleanup: 
static void

nvmf_rdma_qpair_clean_ibv_events(struct spdk_nvmf_rdma_qpair *rqpair)

{

	struct spdk_nvmf_rdma_ibv_event_ctx *ctx, *tctx;

	STAILQ_FOREACH_SAFE(ctx, &rqpair->ibv_events, link, tctx) {

	struct spdk_nvmf_rdma_ibv_event_ctx *ctx;



	ctx = rqpair->last_wqe_reached_ctx;

	if (ctx) {

		ctx->rqpair = NULL;

		/* Memory allocated for ctx is freed in nvmf_rdma_qpair_process_last_wqe_event */

		STAILQ_REMOVE(&rqpair->ibv_events, ctx, spdk_nvmf_rdma_ibv_event_ctx, link);

	}

}
@@ -1378,7 +1377,6 @@ nvmf_rdma_connect(struct spdk_nvmf_transport *transport, struct rdma_cm_event *e 
	rqpair->cm_id = event->id;

	rqpair->listen_id = event->listen_id;

	rqpair->qpair.transport = transport;

	STAILQ_INIT(&rqpair->ibv_events);

	/* use qid from the private data to determine the qpair type

	   qid will be set to the appropriate value when the controller is created */

	rqpair->qpair.qid = private_data->qid;
@@ -3744,10 +3742,14 @@ static void 
nvmf_rdma_qpair_process_last_wqe_event(void *ctx)

{

	struct spdk_nvmf_rdma_ibv_event_ctx *event_ctx = ctx;

	struct spdk_nvmf_rdma_qpair *rqpair;



	if (event_ctx->rqpair) {

		STAILQ_REMOVE(&event_ctx->rqpair->ibv_events, event_ctx, spdk_nvmf_rdma_ibv_event_ctx, link);

		nvmf_rdma_handle_last_wqe_reached(event_ctx->rqpair);

	rqpair = event_ctx->rqpair;



	if (rqpair) {

		assert(event_ctx == rqpair->last_wqe_reached_ctx);

		nvmf_rdma_handle_last_wqe_reached(rqpair);

		rqpair->last_wqe_reached_ctx = NULL;

	}

	free(event_ctx);

}
@@ -3770,17 +3772,22 @@ nvmf_rdma_send_qpair_last_wqe_event(struct spdk_nvmf_rdma_qpair *rqpair) 
		return -EINVAL;

	}



	if (rqpair->last_wqe_reached || rqpair->last_wqe_reached_ctx != NULL) {

		SPDK_ERRLOG("LAST_WQE_REACHED already received for rqpair %p\n", rqpair);

		return -EALREADY;

	}



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

	if (!ctx) {

		return -ENOMEM;

	}



	ctx->rqpair = rqpair;

	STAILQ_INSERT_TAIL(&rqpair->ibv_events, ctx, link);

	rqpair->last_wqe_reached_ctx = ctx;



	rc = spdk_thread_send_msg(thr, nvmf_rdma_qpair_process_last_wqe_event, ctx);

	if (rc) {

		STAILQ_REMOVE(&rqpair->ibv_events, ctx, spdk_nvmf_rdma_ibv_event_ctx, link);

		rqpair->last_wqe_reached_ctx = NULL;

		free(ctx);

	}