nvme: factor out process lookup into a function

Change-Id: I7598222db5d76c1a1578fbb5935d4348f7c62f54
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/410951
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>

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) {
+	active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr);
-		if (active_proc->pid == pid) {
+	if (active_proc) {
-			TAILQ_INSERT_TAIL(&active_proc->allocated_io_qpairs, qpair,
+		TAILQ_INSERT_TAIL(&active_proc->allocated_io_qpairs, qpair, per_process_tailq);
-					  per_process_tailq);
 		qpair->active_proc = active_proc;
-			break;
-		}
 	}
 }
 
@@ -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) {
+	active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr);
-		if (active_proc->pid == pid) {
+	if (!active_proc) {
-			proc_found = true;
-			break;
-		}
-	}
-
-	if (proc_found == false) {
 		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,15 +1279,13 @@ 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 (spdk_nvme_ctrlr_get_process(ctrlr, pid)) {
-		if (active_proc->pid == pid) {
 		return 0;
 	}
-	}
 
 	/* Initialize the per process properties for this ctrlr */
 	ctrlr_proc = spdk_dma_zmalloc(sizeof(struct spdk_nvme_ctrlr_process), 64, NULL);
@@ -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) {
+	active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr);
-		if (active_proc->pid == pid) {
+	if (active_proc) {
 		active_proc->ref++;
-			break;
-		}
 	}
 
 	nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock);
@@ -1430,16 +1433,15 @@ 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;
+	struct spdk_nvme_ctrlr_process	*active_proc;
-	pid_t				pid = getpid();
 	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) {
+	active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr);
-		if (active_proc->pid == pid) {
+	if (active_proc) {
 		active_proc->ref--;
 		assert(active_proc->ref >= 0);
 
@@ -1450,9 +1452,6 @@ nvme_ctrlr_proc_put_ref(struct spdk_nvme_ctrlr *ctrlr)
 		if (active_proc->ref == 0 && proc_count != 1) {
 			nvme_ctrlr_remove_process(ctrlr, active_proc);
 		}
-
-			break;
-		}
 	}
 
 	nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock);
@@ -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) {
+	active_proc = spdk_nvme_ctrlr_get_current_process(ctrlr);
-		if (active_proc->pid == pid) {
+	if (active_proc) {
 		devhandle = active_proc->devhandle;
-			break;
-		}
 	}
 
 	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;
+	struct spdk_nvme_ctrlr_process	*active_proc;
-	pid_t				pid = getpid();
-
-	TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) {
-		if (active_proc->pid == pid) {
-			break;
-		}
-	}
-
-	assert(active_proc != NULL);
 
+	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;
+	}
 }
 
 bool

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);

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) {
+	active_proc = spdk_nvme_ctrlr_get_process(ctrlr, active_req->pid);
-		if (active_proc->pid == active_req->pid) {
+	if (active_proc) {
-			/* Saved the original completion information */
+		/* Save 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;
+	} else {
-
+		SPDK_ERRLOG("The owning process (pid %d) is not found. Dropping the request.\n",
-			break;
-		}
-	}
-
-	if (pending_on_proc == false) {
-		SPDK_ERRLOG("The owning process (pid %d) is not found. Drop 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) {
+	proc = spdk_nvme_ctrlr_get_current_process(ctrlr);
-		if (proc->pid == pid) {
+	if (!proc) {
-			proc_found = true;
-
-			break;
-		}
-	}
-
-	if (proc_found == false) {
 		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) {

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,