ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

nvme_tcp: fix memory leak when resetting controllor

Browse Source 
In failover test, it reports memory leak about tqpair->stats when
detaching a tcp controller and it failover to the other controller.

Because during resetting the controller, we disconnect the controller
at first and then reconnect. when disconnecting, the adminq is not
freed which means the corresponding tqpair and tqpair->stats are not
freed. But when reconnecting, nvme_tcp_ctrlr_connect_qpair will
allocate memory for tqpair->stats again which causes memory leak.

So this patch fix the bug by not reallocating memory for tqpair->stats
if it's not NULL. We keep the old stats because from user perspective,
the spdk_nvme_qpair is the same one.

Besides, when destroying a qpair, the qpair->poll_group is set as
NULL which means if qpair->poll_group is not NULL, it should be a
new qpair. So there's no need to check if stats is NULL or not if
qpair->poll_group is not NULL. So adjusting the if...else... in
_nvme_pcie_ctrlr_create_io_qpair.

Change-Id: I4108a980aeffe4797e5bca5b1a8ea89f7457162b
Signed-off-by: Richael Zhuang <richael.zhuang@arm.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17718
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Krzysztof Karas <krzysztof.karas@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This commit is contained in:
Richael Zhuang 2023-04-23 10:42:53 +08:00 committed by David Ko
parent 7be13fbd61
commit 46113de13b
2 changed files with 17 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
lib/nvme/nvme_pcie_common.c
View File

@ -530,14 +530,14 @@ _nvme_pcie_ctrlr_create_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme
int rc; int rc;
/* Statistics may already be allocated in the case of controller reset */ /* Statistics may already be allocated in the case of controller reset */
if (!pqpair->stat) { if (qpair->poll_group) {
if (qpair->poll_group) { struct nvme_pcie_poll_group *group = SPDK_CONTAINEROF(qpair->poll_group,
struct nvme_pcie_poll_group *group = SPDK_CONTAINEROF(qpair->poll_group, struct nvme_pcie_poll_group, group);
struct nvme_pcie_poll_group, group);
pqpair->stat = &group->stats; pqpair->stat = &group->stats;
pqpair->shared_stats = true; pqpair->shared_stats = true;
} else { } else {
if (pqpair->stat == NULL) {
pqpair->stat = calloc(1, sizeof(*pqpair->stat)); pqpair->stat = calloc(1, sizeof(*pqpair->stat));
if (!pqpair->stat) { if (!pqpair->stat) {
SPDK_ERRLOG("Failed to allocate qpair statistics\n"); SPDK_ERRLOG("Failed to allocate qpair statistics\n");
@ -547,7 +547,6 @@ _nvme_pcie_ctrlr_create_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme
} }
} }
rc = nvme_pcie_ctrlr_cmd_create_io_cq(ctrlr, qpair, nvme_completion_create_cq_cb, qpair); rc = nvme_pcie_ctrlr_cmd_create_io_cq(ctrlr, qpair, nvme_completion_create_cq_cb, qpair);
if (rc != 0) { if (rc != 0) {

14
lib/nvme/nvme_tcp.c
View File

@ -2048,10 +2048,16 @@ nvme_tcp_ctrlr_connect_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_qpa
tqpair->stats = &tgroup->stats; tqpair->stats = &tgroup->stats;
tqpair->shared_stats = true; tqpair->shared_stats = true;
} else { } else {
tqpair->stats = calloc(1, sizeof(*tqpair->stats)); /* When resetting a controller, we disconnect adminq and then reconnect. The stats
if (!tqpair->stats) { * is not freed when disconnecting. So when reconnecting, don't allocate memory
SPDK_ERRLOG("tcp stats memory allocation failed\n"); * again.
return -ENOMEM; */
if (tqpair->stats == NULL) {
tqpair->stats = calloc(1, sizeof(*tqpair->stats));
if (!tqpair->stats) {
SPDK_ERRLOG("tcp stats memory allocation failed\n");
return -ENOMEM;
}
} }
} }