From 20f92ad45a39dc793efc8fb3897d352962728325 Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Thu, 31 Mar 2016 21:00:25 +0800 Subject: [PATCH] SPDK: Handle the memory leak issue for nvme_request free This patch is used to handle the memory leak issue when a parent nvme_request is free. In our current code, we did not free the nvme_request allocated by the children in the exceptional case. Change-Id: Iabd1f1c3594af60c38e74e3d96c14f78d1aa1aed Signed-off-by: Ziye Yang --- lib/nvme/nvme_ns_cmd.c | 11 +++++++++-- lib/nvme/nvme_qpair.c | 20 +++++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/nvme/nvme_ns_cmd.c b/lib/nvme/nvme_ns_cmd.c index 889b266da..55ef8fc7d 100644 --- a/lib/nvme/nvme_ns_cmd.c +++ b/lib/nvme/nvme_ns_cmd.c @@ -91,7 +91,7 @@ _nvme_ns_cmd_split_request(struct spdk_nvme_ns *ns, uint32_t sector_size = ns->sector_size; uint32_t remaining_lba_count = lba_count; uint32_t offset = 0; - struct nvme_request *child; + struct nvme_request *child, *tmp; while (remaining_lba_count > 0) { lba_count = sectors_per_max_io - (lba & sector_mask); @@ -100,7 +100,14 @@ _nvme_ns_cmd_split_request(struct spdk_nvme_ns *ns, child = _nvme_ns_cmd_rw(ns, payload, lba, lba_count, cb_fn, cb_arg, opc, io_flags); if (child == NULL) { - nvme_free_request(req); + if (req->num_children) { + /* free all child nvme_request */ + TAILQ_FOREACH_SAFE(child, &req->children, + child_tailq, tmp) { + nvme_remove_child_request(req, child); + nvme_free_request(child); + } + } return NULL; } child->payload_offset = offset; diff --git a/lib/nvme/nvme_qpair.c b/lib/nvme/nvme_qpair.c index 85bbca890..2137151f5 100644 --- a/lib/nvme/nvme_qpair.c +++ b/lib/nvme/nvme_qpair.c @@ -813,10 +813,11 @@ _nvme_qpair_build_prps_sgl_request(struct spdk_nvme_qpair *qpair, struct nvme_re int nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *req) { - int rc; + int rc = 0; struct nvme_tracker *tr; - struct nvme_request *child_req; + struct nvme_request *child_req, *tmp; struct spdk_nvme_ctrlr *ctrlr = qpair->ctrlr; + bool child_req_failed = false; if (ctrlr->is_failed) { nvme_free_request(req); @@ -830,13 +831,18 @@ nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *re * This is a split (parent) request. Submit all of the children but not the parent * request itself, since the parent is the original unsplit request. */ - TAILQ_FOREACH(child_req, &req->children, child_tailq) { - rc = nvme_qpair_submit_request(qpair, child_req); - if (rc != 0) { - return rc; + TAILQ_FOREACH_SAFE(child_req, &req->children, child_tailq, tmp) { + if (!child_req_failed) { + rc = nvme_qpair_submit_request(qpair, child_req); + if (rc != 0) + child_req_failed = true; + } else { /* free remaining child_reqs since one child_req fails */ + nvme_remove_child_request(req, child_req); + nvme_free_request(child_req); } } - return 0; + + return rc; } tr = LIST_FIRST(&qpair->free_tr);