From f1941efe7beae43ee09203b03cf6757fe91de43e Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 21 Dec 2021 10:33:37 +0900 Subject: [PATCH] nvme_tcp: Use dummy stats after removing qpair from poll group Previously, when connecting qpair, we allocated stats per qpair if poll group is not used or we set stats per poll group otherwise. Then when removing qpair from poll group, we cleared qpair->stats pointer. However, if qpair is still not completely disconnected after removing qpair from poll group, tqpair->stats is NULL and it causes a segmentation fault. Hence we set tqpair->stats to &g_dummy_stats instead of NULL. Signed-off-by: Shuhei Matsumoto Change-Id: Ice6469627ce8d4bf4567f57c304759206b6432f1 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10844 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Jim Harris --- lib/nvme/nvme_tcp.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index e9489796d..5cfe96543 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -115,6 +115,8 @@ struct nvme_tcp_qpair { bool needs_poll; uint64_t icreq_timeout_tsc; + + bool shared_stats; }; enum nvme_tcp_req_state { @@ -166,6 +168,8 @@ struct nvme_tcp_req { struct spdk_nvme_cpl rsp; }; +static struct spdk_nvme_tcp_stat g_dummy_stats = {}; + static void nvme_tcp_send_h2c_data(struct nvme_tcp_req *tcp_req); static int64_t nvme_tcp_poll_group_process_completions(struct spdk_nvme_transport_poll_group *tgroup, uint32_t completions_per_qpair, spdk_nvme_disconnected_qpair_cb disconnected_qpair_cb); @@ -354,7 +358,9 @@ nvme_tcp_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_q nvme_qpair_deinit(qpair); tqpair = nvme_tcp_qpair(qpair); nvme_tcp_free_reqs(tqpair); - free(tqpair->stats); + if (!tqpair->shared_stats) { + free(tqpair->stats); + } free(tqpair); return 0; @@ -2012,6 +2018,7 @@ nvme_tcp_ctrlr_connect_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_qpa } tgroup = nvme_tcp_poll_group(qpair->poll_group); tqpair->stats = &tgroup->stats; + tqpair->shared_stats = true; } else { tqpair->stats = calloc(1, sizeof(*tqpair->stats)); if (!tqpair->stats) { @@ -2284,9 +2291,9 @@ nvme_tcp_poll_group_remove(struct spdk_nvme_transport_poll_group *tgroup, } tqpair = nvme_tcp_qpair(qpair); - /* When qpair is deleted, stats are freed. free(NULL) is valid case, so just set - * stats pointer to NULL */ - tqpair->stats = NULL; + + assert(tqpair->shared_stats == true); + tqpair->stats = &g_dummy_stats; return rc; }