diff --git a/lib/nvme/nvme.c b/lib/nvme/nvme.c index 41af20fc2..dcbe85d29 100644 --- a/lib/nvme/nvme.c +++ b/lib/nvme/nvme.c @@ -62,8 +62,12 @@ spdk_nvme_detach(struct spdk_nvme_ctrlr *ctrlr) { pthread_mutex_lock(&g_spdk_nvme_driver->lock); - TAILQ_REMOVE(&g_spdk_nvme_driver->attached_ctrlrs, ctrlr, tailq); - nvme_ctrlr_destruct(ctrlr); + nvme_ctrlr_proc_put_ref(ctrlr); + + if (nvme_ctrlr_get_ref_count(ctrlr) == 0) { + TAILQ_REMOVE(&g_spdk_nvme_driver->attached_ctrlrs, ctrlr, tailq); + nvme_ctrlr_destruct(ctrlr); + } pthread_mutex_unlock(&g_spdk_nvme_driver->lock); return 0; @@ -326,6 +330,12 @@ spdk_nvme_probe(void *cb_ctx, spdk_nvme_probe_cb probe_cb, spdk_nvme_attach_cb a TAILQ_REMOVE(&g_spdk_nvme_driver->init_ctrlrs, ctrlr, tailq); TAILQ_INSERT_TAIL(&g_spdk_nvme_driver->attached_ctrlrs, ctrlr, tailq); + /* + * Increase the ref count before calling attach_cb() as the user may + * call nvme_detach() immediately. + */ + nvme_ctrlr_proc_get_ref(ctrlr); + /* * Unlock while calling attach_cb() so the user can call other functions * that may take the driver lock, like nvme_detach(). diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index ed5e56937..a5d0074bd 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -33,6 +33,7 @@ #include "nvme_internal.h" #include "spdk/env.h" +#include static int nvme_ctrlr_construct_and_submit_aer(struct spdk_nvme_ctrlr *ctrlr, struct nvme_async_event_request *aer); @@ -822,12 +823,34 @@ nvme_ctrlr_add_process(struct spdk_nvme_ctrlr *ctrlr, void *devhandle) ctrlr_proc->pid = getpid(); STAILQ_INIT(&ctrlr_proc->active_reqs); ctrlr_proc->devhandle = devhandle; + ctrlr_proc->ref = 0; TAILQ_INSERT_TAIL(&ctrlr->active_procs, ctrlr_proc, tailq); return 0; } +/** + * This function will be called when the process exited unexpectedly + * in order to free any incomplete nvme request and allocated memory. + * Note: the ctrl_lock must be held when calling this function. + */ +static void +nvme_ctrlr_cleanup_process(struct spdk_nvme_controller_process *proc) +{ + struct nvme_request *req, *tmp_req; + + STAILQ_FOREACH_SAFE(req, &proc->active_reqs, stailq, tmp_req) { + STAILQ_REMOVE(&proc->active_reqs, req, nvme_request, stailq); + + assert(req->pid == proc->pid); + + nvme_free_request(req); + } + + spdk_free(proc); +} + /** * This function will be called when destructing the controller. * 1. There is no more admin request on this controller. @@ -848,6 +871,88 @@ nvme_ctrlr_free_processes(struct spdk_nvme_ctrlr *ctrlr) } } +/** + * This function will be called when any other process attaches or + * detaches the controller in order to cleanup those unexpectedly + * terminated processes. + * Note: the ctrl_lock must be held when calling this function. + */ +static void +nvme_ctrlr_remove_inactive_proc(struct spdk_nvme_ctrlr *ctrlr) +{ + struct spdk_nvme_controller_process *active_proc, *tmp; + + TAILQ_FOREACH_SAFE(active_proc, &ctrlr->active_procs, tailq, tmp) { + if ((kill(active_proc->pid, 0) == -1) && (errno == ESRCH)) { + SPDK_ERRLOG("process %d terminated unexpected\n", active_proc->pid); + + TAILQ_REMOVE(&ctrlr->active_procs, active_proc, tailq); + + nvme_ctrlr_cleanup_process(active_proc); + } + } +} + +void +nvme_ctrlr_proc_get_ref(struct spdk_nvme_ctrlr *ctrlr) +{ + struct spdk_nvme_controller_process *active_proc; + pid_t pid = getpid(); + + pthread_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; + } + } + + pthread_mutex_unlock(&ctrlr->ctrlr_lock); +} + +void +nvme_ctrlr_proc_put_ref(struct spdk_nvme_ctrlr *ctrlr) +{ + struct spdk_nvme_controller_process *active_proc; + pid_t pid = getpid(); + + pthread_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--; + assert(active_proc->ref >= 0); + break; + } + } + + pthread_mutex_unlock(&ctrlr->ctrlr_lock); +} + +int +nvme_ctrlr_get_ref_count(struct spdk_nvme_ctrlr *ctrlr) +{ + struct spdk_nvme_controller_process *active_proc; + int ref = 0; + + pthread_mutex_lock(&ctrlr->ctrlr_lock); + + nvme_ctrlr_remove_inactive_proc(ctrlr); + + TAILQ_FOREACH(active_proc, &ctrlr->active_procs, tailq) { + ref += active_proc->ref; + } + + pthread_mutex_unlock(&ctrlr->ctrlr_lock); + + return ref; +} + /** * This function will be called repeatedly during initialization until the controller is ready. */ diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index c8a8cfa72..57ba25926 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -310,6 +310,9 @@ struct spdk_nvme_controller_process { /** Per process PCI device handle */ struct spdk_pci_device *devhandle; + + /** Reference to track the number of attachment to this controller. */ + int ref; }; /* @@ -548,4 +551,14 @@ DECLARE_TRANSPORT(pcie) #undef DECLARE_TRANSPORT +/* + * Below ref related functions must be called with the global + * driver lock held for the multi-process condition. + * Within these functions, the per ctrlr ctrlr_lock is also + * acquired for the multi-thread condition. + */ +void nvme_ctrlr_proc_get_ref(struct spdk_nvme_ctrlr *ctrlr); +void nvme_ctrlr_proc_put_ref(struct spdk_nvme_ctrlr *ctrlr); +int nvme_ctrlr_get_ref_count(struct spdk_nvme_ctrlr *ctrlr); + #endif /* __NVME_INTERNAL_H__ */ diff --git a/test/lib/nvme/unit/nvme_c/nvme_ut.c b/test/lib/nvme/unit/nvme_c/nvme_ut.c index 000c8dc06..fa6ab8996 100644 --- a/test/lib/nvme/unit/nvme_c/nvme_ut.c +++ b/test/lib/nvme/unit/nvme_c/nvme_ut.c @@ -107,6 +107,24 @@ spdk_pci_addr_compare(const struct spdk_pci_addr *a1, const struct spdk_pci_addr return true; } +void +nvme_ctrlr_proc_get_ref(struct spdk_nvme_ctrlr *ctrlr) +{ + return; +} + +void +nvme_ctrlr_proc_put_ref(struct spdk_nvme_ctrlr *ctrlr) +{ + return; +} + +int +nvme_ctrlr_get_ref_count(struct spdk_nvme_ctrlr *ctrlr) +{ + return 0; +} + static void test_opc_data_transfer(void) { diff --git a/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c b/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c index 56f6bbc0f..763dd5b5f 100644 --- a/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c +++ b/test/lib/nvme/unit/nvme_ctrlr_c/nvme_ctrlr_ut.c @@ -388,6 +388,12 @@ nvme_allocate_request_null(spdk_nvme_cmd_cb cb_fn, void *cb_arg) return nvme_allocate_request_contig(NULL, 0, cb_fn, cb_arg); } +void +nvme_free_request(struct nvme_request *req) +{ + spdk_mempool_put(_g_nvme_driver.request_mempool, req); +} + static void test_nvme_ctrlr_init_en_1_rdy_0(void) { diff --git a/test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c b/test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c index e911fa34d..a9ff1966f 100644 --- a/test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c +++ b/test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c @@ -136,6 +136,24 @@ nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *re return 0; } +void +nvme_ctrlr_proc_get_ref(struct spdk_nvme_ctrlr *ctrlr) +{ + return; +} + +void +nvme_ctrlr_proc_put_ref(struct spdk_nvme_ctrlr *ctrlr) +{ + return; +} + +int +nvme_ctrlr_get_ref_count(struct spdk_nvme_ctrlr *ctrlr) +{ + return 0; +} + static void prepare_for_test(struct spdk_nvme_ns *ns, struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_qpair *qpair,