From cbd9c241dcdf9c43389637a7d12ae1e1f70bd4fd Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 11 May 2018 14:46:55 -0700 Subject: [PATCH] nvme: factor out process lookup into a function Change-Id: I7598222db5d76c1a1578fbb5935d4348f7c62f54 Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/410951 Tested-by: SPDK Automated Test System Reviewed-by: Changpeng Liu Reviewed-by: Ben Walker --- lib/nvme/nvme_ctrlr.c | 114 ++++++++---------- lib/nvme/nvme_internal.h | 3 + lib/nvme/nvme_pcie.c | 36 ++---- test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c | 10 ++ 4 files changed, 76 insertions(+), 87 deletions(-) diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 3c02d8759..37d449b5f 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -146,15 +146,11 @@ nvme_ctrlr_proc_add_io_qpair(struct spdk_nvme_qpair *qpair) { struct spdk_nvme_ctrlr_process *active_proc; struct spdk_nvme_ctrlr *ctrlr = qpair->ctrlr; - pid_t pid = getpid(); - TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) { - if (active_proc->pid == pid) { - TAILQ_INSERT_TAIL(&active_proc->allocated_io_qpairs, qpair, - per_process_tailq); - qpair->active_proc = active_proc; - break; - } + active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr); + if (active_proc) { + TAILQ_INSERT_TAIL(&active_proc->allocated_io_qpairs, qpair, per_process_tailq); + qpair->active_proc = active_proc; } } @@ -168,17 +164,9 @@ nvme_ctrlr_proc_remove_io_qpair(struct spdk_nvme_qpair *qpair) struct spdk_nvme_ctrlr_process *active_proc; struct spdk_nvme_ctrlr *ctrlr = qpair->ctrlr; struct spdk_nvme_qpair *active_qpair, *tmp_qpair; - pid_t pid = getpid(); - bool proc_found = false; - TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) { - if (active_proc->pid == pid) { - proc_found = true; - break; - } - } - - if (proc_found == false) { + active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr); + if (!active_proc) { return; } @@ -1262,6 +1250,26 @@ nvme_ctrlr_configure_aer(struct spdk_nvme_ctrlr *ctrlr) return 0; } +struct spdk_nvme_ctrlr_process * +spdk_nvme_ctrlr_get_process(struct spdk_nvme_ctrlr *ctrlr, pid_t pid) +{ + struct spdk_nvme_ctrlr_process *active_proc; + + TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) { + if (active_proc->pid == pid) { + return active_proc; + } + } + + return NULL; +} + +struct spdk_nvme_ctrlr_process * +spdk_nvme_ctrlr_get_current_process(struct spdk_nvme_ctrlr *ctrlr) +{ + return spdk_nvme_ctrlr_get_process(ctrlr, getpid()); +} + /** * This function will be called when a process is using the controller. * 1. For the primary process, it is called when constructing the controller. @@ -1271,14 +1279,12 @@ nvme_ctrlr_configure_aer(struct spdk_nvme_ctrlr *ctrlr) int nvme_ctrlr_add_process(struct spdk_nvme_ctrlr *ctrlr, void *devhandle) { - struct spdk_nvme_ctrlr_process *ctrlr_proc, *active_proc; + struct spdk_nvme_ctrlr_process *ctrlr_proc; pid_t pid = getpid(); /* Check whether the process is already added or not */ - TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) { - if (active_proc->pid == pid) { - return 0; - } + if (spdk_nvme_ctrlr_get_process(ctrlr, pid)) { + return 0; } /* Initialize the per process properties for this ctrlr */ @@ -1411,17 +1417,14 @@ void nvme_ctrlr_proc_get_ref(struct spdk_nvme_ctrlr *ctrlr) { struct spdk_nvme_ctrlr_process *active_proc; - pid_t pid = getpid(); nvme_robust_mutex_lock(&ctrlr->ctrlr_lock); nvme_ctrlr_remove_inactive_proc(ctrlr); - TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) { - if (active_proc->pid == pid) { - active_proc->ref++; - break; - } + active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr); + if (active_proc) { + active_proc->ref++; } nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); @@ -1430,28 +1433,24 @@ nvme_ctrlr_proc_get_ref(struct spdk_nvme_ctrlr *ctrlr) void nvme_ctrlr_proc_put_ref(struct spdk_nvme_ctrlr *ctrlr) { - struct spdk_nvme_ctrlr_process *active_proc, *tmp; - pid_t pid = getpid(); + struct spdk_nvme_ctrlr_process *active_proc; int proc_count; nvme_robust_mutex_lock(&ctrlr->ctrlr_lock); proc_count = nvme_ctrlr_remove_inactive_proc(ctrlr); - TAILQ_FOREACH_SAFE(active_proc, &ctrlr->active_procs, tailq, tmp) { - if (active_proc->pid == pid) { - active_proc->ref--; - assert(active_proc->ref >= 0); + active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr); + if (active_proc) { + active_proc->ref--; + assert(active_proc->ref >= 0); - /* - * The last active process will be removed at the end of - * the destruction of the controller. - */ - if (active_proc->ref == 0 && proc_count != 1) { - nvme_ctrlr_remove_process(ctrlr, active_proc); - } - - break; + /* + * The last active process will be removed at the end of + * the destruction of the controller. + */ + if (active_proc->ref == 0 && proc_count != 1) { + nvme_ctrlr_remove_process(ctrlr, active_proc); } } @@ -1484,16 +1483,13 @@ struct spdk_pci_device * nvme_ctrlr_proc_get_devhandle(struct spdk_nvme_ctrlr *ctrlr) { struct spdk_nvme_ctrlr_process *active_proc; - pid_t pid = getpid(); struct spdk_pci_device *devhandle = NULL; nvme_robust_mutex_lock(&ctrlr->ctrlr_lock); - TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) { - if (active_proc->pid == pid) { - devhandle = active_proc->devhandle; - break; - } + active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr); + if (active_proc) { + devhandle = active_proc->devhandle; } nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); @@ -1988,20 +1984,14 @@ void spdk_nvme_ctrlr_register_timeout_callback(struct spdk_nvme_ctrlr *ctrlr, uint32_t nvme_timeout, spdk_nvme_timeout_cb cb_fn, void *cb_arg) { - struct spdk_nvme_ctrlr_process *active_proc = NULL; - pid_t pid = getpid(); + struct spdk_nvme_ctrlr_process *active_proc; - TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) { - if (active_proc->pid == pid) { - break; - } + active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr); + if (active_proc) { + active_proc->timeout_ticks = nvme_timeout * spdk_get_ticks_hz(); + active_proc->timeout_cb_fn = cb_fn; + active_proc->timeout_cb_arg = cb_arg; } - - assert(active_proc != NULL); - - active_proc->timeout_ticks = nvme_timeout * spdk_get_ticks_hz(); - active_proc->timeout_cb_fn = cb_fn; - active_proc->timeout_cb_arg = cb_arg; } bool diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 01496be9d..3c5083fa1 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -576,6 +576,9 @@ int nvme_ctrlr_cmd_fw_image_download(struct spdk_nvme_ctrlr *ctrlr, spdk_nvme_cmd_cb cb_fn, void *cb_arg); void nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl); +struct spdk_nvme_ctrlr_process *spdk_nvme_ctrlr_get_process(struct spdk_nvme_ctrlr *ctrlr, + pid_t pid); +struct spdk_nvme_ctrlr_process *spdk_nvme_ctrlr_get_current_process(struct spdk_nvme_ctrlr *ctrlr); int nvme_ctrlr_add_process(struct spdk_nvme_ctrlr *ctrlr, void *devhandle); void nvme_ctrlr_free_processes(struct spdk_nvme_ctrlr *ctrlr); struct spdk_pci_device *nvme_ctrlr_proc_get_devhandle(struct spdk_nvme_ctrlr *ctrlr); diff --git a/lib/nvme/nvme_pcie.c b/lib/nvme/nvme_pcie.c index c41dea453..40fe21e11 100644 --- a/lib/nvme/nvme_pcie.c +++ b/lib/nvme/nvme_pcie.c @@ -1079,7 +1079,6 @@ nvme_pcie_qpair_insert_pending_admin_request(struct spdk_nvme_qpair *qpair, struct spdk_nvme_ctrlr *ctrlr = qpair->ctrlr; struct nvme_request *active_req = req; struct spdk_nvme_ctrlr_process *active_proc; - bool pending_on_proc = false; /* * The admin request is from another process. Move to the per @@ -1088,19 +1087,13 @@ nvme_pcie_qpair_insert_pending_admin_request(struct spdk_nvme_qpair *qpair, assert(nvme_qpair_is_admin_queue(qpair)); assert(active_req->pid != getpid()); - TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) { - if (active_proc->pid == active_req->pid) { - /* Saved the original completion information */ - memcpy(&active_req->cpl, cpl, sizeof(*cpl)); - STAILQ_INSERT_TAIL(&active_proc->active_reqs, active_req, stailq); - pending_on_proc = true; - - break; - } - } - - if (pending_on_proc == false) { - SPDK_ERRLOG("The owning process (pid %d) is not found. Drop the request.\n", + active_proc = spdk_nvme_ctrlr_get_process(ctrlr, active_req->pid); + if (active_proc) { + /* Save the original completion information */ + memcpy(&active_req->cpl, cpl, sizeof(*cpl)); + STAILQ_INSERT_TAIL(&active_proc->active_reqs, active_req, stailq); + } else { + SPDK_ERRLOG("The owning process (pid %d) is not found. Dropping the request.\n", active_req->pid); nvme_free_request(active_req); @@ -1115,7 +1108,6 @@ nvme_pcie_qpair_complete_pending_admin_request(struct spdk_nvme_qpair *qpair) { struct spdk_nvme_ctrlr *ctrlr = qpair->ctrlr; struct nvme_request *req, *tmp_req; - bool proc_found = false; pid_t pid = getpid(); struct spdk_nvme_ctrlr_process *proc; @@ -1125,17 +1117,11 @@ nvme_pcie_qpair_complete_pending_admin_request(struct spdk_nvme_qpair *qpair) */ assert(nvme_qpair_is_admin_queue(qpair)); - TAILQ_FOREACH(proc, &ctrlr->active_procs, tailq) { - if (proc->pid == pid) { - proc_found = true; - - break; - } - } - - if (proc_found == false) { + proc = spdk_nvme_ctrlr_get_current_process(ctrlr); + if (!proc) { SPDK_ERRLOG("the active process (pid %d) is not found for this controller.\n", pid); - assert(proc_found); + assert(proc); + return; } STAILQ_FOREACH_SAFE(req, &proc->active_reqs, stailq, tmp_req) { diff --git a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c index 36abba061..2f95b65d4 100644 --- a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c +++ b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c @@ -42,6 +42,16 @@ DEFINE_STUB(spdk_mem_register, int, (void *vaddr, size_t len), 0); DEFINE_STUB(spdk_mem_unregister, int, (void *vaddr, size_t len), 0); +DEFINE_STUB(spdk_nvme_ctrlr_get_process, + struct spdk_nvme_ctrlr_process *, + (struct spdk_nvme_ctrlr *ctrlr, pid_t pid), + NULL); + +DEFINE_STUB(spdk_nvme_ctrlr_get_current_process, + struct spdk_nvme_ctrlr_process *, + (struct spdk_nvme_ctrlr *ctrlr), + NULL); + struct spdk_trace_flag SPDK_LOG_NVME = { .name = "nvme", .enabled = false,