Date: Thu, 20 Feb 2025 15:40:41 GMT From: John Baldwin <jhb@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 97ca2ada80b8 - main - nvmft: Switch the per-port lock from sx(9) to mtx(9) Message-ID: <202502201540.51KFef5Y016893@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=97ca2ada80b870edbbb4f66b26e274cf8e55e0bc commit 97ca2ada80b870edbbb4f66b26e274cf8e55e0bc Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2025-02-20 15:14:16 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2025-02-20 15:31:20 +0000 nvmft: Switch the per-port lock from sx(9) to mtx(9) This is needed to avoid LORs for a following commit. Sponsored by: Chelsio Communications --- sys/dev/nvmf/controller/ctl_frontend_nvmf.c | 58 ++++++++++++++++------------- sys/dev/nvmf/controller/nvmft_controller.c | 46 ++++++++++++++--------- sys/dev/nvmf/controller/nvmft_var.h | 2 +- 3 files changed, 62 insertions(+), 44 deletions(-) diff --git a/sys/dev/nvmf/controller/ctl_frontend_nvmf.c b/sys/dev/nvmf/controller/ctl_frontend_nvmf.c index fcfa8b90ebb7..a28a613e23aa 100644 --- a/sys/dev/nvmf/controller/ctl_frontend_nvmf.c +++ b/sys/dev/nvmf/controller/ctl_frontend_nvmf.c @@ -70,9 +70,9 @@ nvmft_online(void *arg) { struct nvmft_port *np = arg; - sx_xlock(&np->lock); + mtx_lock(&np->lock); np->online = true; - sx_xunlock(&np->lock); + mtx_unlock(&np->lock); } static void @@ -81,7 +81,7 @@ nvmft_offline(void *arg) struct nvmft_port *np = arg; struct nvmft_controller *ctrlr; - sx_xlock(&np->lock); + mtx_lock(&np->lock); np->online = false; TAILQ_FOREACH(ctrlr, &np->controllers, link) { @@ -91,8 +91,8 @@ nvmft_offline(void *arg) } while (!TAILQ_EMPTY(&np->controllers)) - sx_sleep(np, &np->lock, 0, "nvmfoff", 0); - sx_xunlock(&np->lock); + mtx_sleep(np, &np->lock, 0, "nvmfoff", 0); + mtx_unlock(&np->lock); } static int @@ -102,7 +102,7 @@ nvmft_lun_enable(void *arg, int lun_id) struct nvmft_controller *ctrlr; uint32_t *old_ns, *new_ns; uint32_t nsid; - u_int i; + u_int i, new_count; if (lun_id >= le32toh(np->cdata.nn)) { printf("NVMFT: %s lun %d larger than maximum nsid %u\n", @@ -111,14 +111,22 @@ nvmft_lun_enable(void *arg, int lun_id) } nsid = lun_id + 1; - sx_xlock(&np->lock); - new_ns = mallocarray(np->num_ns + 1, sizeof(*new_ns), M_NVMFT, - M_WAITOK); + mtx_lock(&np->lock); + for (;;) { + new_count = np->num_ns + 1; + mtx_unlock(&np->lock); + new_ns = mallocarray(new_count, sizeof(*new_ns), M_NVMFT, + M_WAITOK); + mtx_lock(&np->lock); + if (np->num_ns + 1 <= new_count) + break; + free(new_ns, M_NVMFT); + } for (i = 0; i < np->num_ns; i++) { if (np->active_ns[i] < nsid) continue; if (np->active_ns[i] == nsid) { - sx_xunlock(&np->lock); + mtx_unlock(&np->lock); free(new_ns, M_NVMFT); printf("NVMFT: %s duplicate lun %d\n", np->cdata.subnqn, lun_id); @@ -145,7 +153,7 @@ nvmft_lun_enable(void *arg, int lun_id) nvmft_controller_lun_changed(ctrlr, lun_id); } - sx_xunlock(&np->lock); + mtx_unlock(&np->lock); free(old_ns, M_NVMFT); return (0); @@ -163,12 +171,12 @@ nvmft_lun_disable(void *arg, int lun_id) return (0); nsid = lun_id + 1; - sx_xlock(&np->lock); + mtx_lock(&np->lock); for (i = 0; i < np->num_ns; i++) { if (np->active_ns[i] == nsid) goto found; } - sx_xunlock(&np->lock); + mtx_unlock(&np->lock); printf("NVMFT: %s request to disable nonexistent lun %d\n", np->cdata.subnqn, lun_id); return (EINVAL); @@ -185,7 +193,7 @@ found: nvmft_controller_lun_changed(ctrlr, lun_id); } - sx_xunlock(&np->lock); + mtx_unlock(&np->lock); return (0); } @@ -196,7 +204,7 @@ nvmft_populate_active_nslist(struct nvmft_port *np, uint32_t nsid, { u_int i, count; - sx_slock(&np->lock); + mtx_lock(&np->lock); count = 0; for (i = 0; i < np->num_ns; i++) { if (np->active_ns[i] <= nsid) @@ -206,7 +214,7 @@ nvmft_populate_active_nslist(struct nvmft_port *np, uint32_t nsid, if (count == nitems(nslist->ns)) break; } - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); } void @@ -625,7 +633,7 @@ nvmft_port_free(struct nvmft_port *np) free(np->active_ns, M_NVMFT); clean_unrhdr(np->ids); delete_unrhdr(np->ids); - sx_destroy(&np->lock); + mtx_destroy(&np->lock); free(np, M_NVMFT); } @@ -797,7 +805,7 @@ nvmft_port_create(struct ctl_req *req) refcount_init(&np->refs, 1); np->max_io_qsize = max_io_qsize; np->cap = _nvmf_controller_cap(max_io_qsize, enable_timeout / 500); - sx_init(&np->lock, "nvmft port"); + mtx_init(&np->lock, "nvmft port", NULL, MTX_DEF); np->ids = new_unrhdr(0, MIN(CTL_MAX_INIT_PER_PORT - 1, NVMF_CNTLID_STATIC_MAX), UNR_NO_MTX); TAILQ_INIT(&np->controllers); @@ -915,12 +923,12 @@ nvmft_port_remove(struct ctl_req *req) TAILQ_REMOVE(&nvmft_ports, np, link); sx_xunlock(&nvmft_ports_lock); - sx_slock(&np->lock); + mtx_lock(&np->lock); if (np->online) { - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); ctl_port_offline(&np->port); } else - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); nvmft_port_rele(np); req->status = CTL_LUN_OK; @@ -1058,7 +1066,7 @@ nvmft_list(struct ctl_nvmf *cn) sbuf_printf(sb, "<ctlnvmflist>\n"); sx_slock(&nvmft_ports_lock); TAILQ_FOREACH(np, &nvmft_ports, link) { - sx_slock(&np->lock); + mtx_lock(&np->lock); TAILQ_FOREACH(ctrlr, &np->controllers, link) { sbuf_printf(sb, "<connection id=\"%d\">" "<hostnqn>%s</hostnqn>" @@ -1070,7 +1078,7 @@ nvmft_list(struct ctl_nvmf *cn) np->cdata.subnqn, ctrlr->trtype); } - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); } sx_sunlock(&nvmft_ports_lock); sbuf_printf(sb, "</ctlnvmflist>\n"); @@ -1108,7 +1116,7 @@ nvmft_terminate(struct ctl_nvmf *cn) found = false; sx_slock(&nvmft_ports_lock); TAILQ_FOREACH(np, &nvmft_ports, link) { - sx_slock(&np->lock); + mtx_lock(&np->lock); TAILQ_FOREACH(ctrlr, &np->controllers, link) { if (tp->all != 0) match = true; @@ -1126,7 +1134,7 @@ nvmft_terminate(struct ctl_nvmf *cn) nvmft_controller_error(ctrlr, NULL, ECONNABORTED); found = true; } - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); } sx_sunlock(&nvmft_ports_lock); diff --git a/sys/dev/nvmf/controller/nvmft_controller.c b/sys/dev/nvmf/controller/nvmft_controller.c index 83a156d9b92a..96c9fee47357 100644 --- a/sys/dev/nvmf/controller/nvmft_controller.c +++ b/sys/dev/nvmf/controller/nvmft_controller.c @@ -14,7 +14,6 @@ #include <sys/memdesc.h> #include <sys/mutex.h> #include <sys/sbuf.h> -#include <sys/sx.h> #include <sys/taskqueue.h> #include <dev/nvmf/nvmf_transport.h> @@ -55,8 +54,6 @@ nvmft_controller_alloc(struct nvmft_port *np, uint16_t cntlid, ctrlr = malloc(sizeof(*ctrlr), M_NVMFT, M_WAITOK | M_ZERO); ctrlr->cntlid = cntlid; - nvmft_port_ref(np); - TAILQ_INSERT_TAIL(&np->controllers, ctrlr, link); ctrlr->np = np; mtx_init(&ctrlr->lock, "nvmft controller", NULL, MTX_DEF); callout_init(&ctrlr->ka_timer, 1); @@ -126,10 +123,10 @@ nvmft_handoff_admin_queue(struct nvmft_port *np, enum nvmf_trtype trtype, return (ENXIO); } - sx_xlock(&np->lock); + mtx_lock(&np->lock); cntlid = alloc_unr(np->ids); if (cntlid == -1) { - sx_xunlock(&np->lock); + mtx_unlock(&np->lock); printf("NVMFT: Unable to allocate controller for %.*s\n", (int)sizeof(data->hostnqn), data->hostnqn); nvmft_connect_error(qp, cmd, NVME_SCT_COMMAND_SPECIFIC, @@ -144,8 +141,21 @@ nvmft_handoff_admin_queue(struct nvmft_port *np, enum nvmf_trtype trtype, ("%s: duplicate controllers with id %d", __func__, cntlid)); } #endif + mtx_unlock(&np->lock); ctrlr = nvmft_controller_alloc(np, cntlid, data); + + mtx_lock(&np->lock); + if (!np->online) { + mtx_unlock(&np->lock); + nvmft_controller_free(ctrlr); + free_unr(np->ids, cntlid); + nvmft_qpair_destroy(qp); + return (ENXIO); + } + nvmft_port_ref(np); + TAILQ_INSERT_TAIL(&np->controllers, ctrlr, link); + nvmft_printf(ctrlr, "associated with %.*s\n", (int)sizeof(data->hostnqn), data->hostnqn); ctrlr->admin = qp; @@ -165,9 +175,9 @@ nvmft_handoff_admin_queue(struct nvmft_port *np, enum nvmf_trtype trtype, callout_reset_sbt(&ctrlr->ka_timer, ctrlr->ka_sbt, 0, nvmft_keep_alive_timer, ctrlr, C_HARDCLOCK); } + mtx_unlock(&np->lock); nvmft_finish_accept(qp, cmd, ctrlr); - sx_xunlock(&np->lock); return (0); } @@ -195,13 +205,13 @@ nvmft_handoff_io_queue(struct nvmft_port *np, enum nvmf_trtype trtype, return (ENXIO); } - sx_slock(&np->lock); + mtx_lock(&np->lock); TAILQ_FOREACH(ctrlr, &np->controllers, link) { if (ctrlr->cntlid == cntlid) break; } if (ctrlr == NULL) { - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); printf("NVMFT: Nonexistent controller %u for I/O queue %u from %.*s\n", ctrlr->cntlid, qid, (int)sizeof(data->hostnqn), data->hostnqn); @@ -212,7 +222,7 @@ nvmft_handoff_io_queue(struct nvmft_port *np, enum nvmf_trtype trtype, } if (memcmp(ctrlr->hostid, data->hostid, sizeof(ctrlr->hostid)) != 0) { - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); nvmft_printf(ctrlr, "hostid mismatch for I/O queue %u from %.*s\n", qid, (int)sizeof(data->hostnqn), data->hostnqn); @@ -222,7 +232,7 @@ nvmft_handoff_io_queue(struct nvmft_port *np, enum nvmf_trtype trtype, return (EINVAL); } if (memcmp(ctrlr->hostnqn, data->hostnqn, sizeof(ctrlr->hostnqn)) != 0) { - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); nvmft_printf(ctrlr, "hostnqn mismatch for I/O queue %u from %.*s\n", qid, (int)sizeof(data->hostnqn), data->hostnqn); @@ -237,7 +247,7 @@ nvmft_handoff_io_queue(struct nvmft_port *np, enum nvmf_trtype trtype, mtx_lock(&ctrlr->lock); if (ctrlr->shutdown) { mtx_unlock(&ctrlr->lock); - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); nvmft_printf(ctrlr, "attempt to create I/O queue %u on disabled controller from %.*s\n", qid, (int)sizeof(data->hostnqn), data->hostnqn); @@ -248,7 +258,7 @@ nvmft_handoff_io_queue(struct nvmft_port *np, enum nvmf_trtype trtype, } if (ctrlr->num_io_queues == 0) { mtx_unlock(&ctrlr->lock); - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); nvmft_printf(ctrlr, "attempt to create I/O queue %u without enabled queues from %.*s\n", qid, (int)sizeof(data->hostnqn), data->hostnqn); @@ -259,7 +269,7 @@ nvmft_handoff_io_queue(struct nvmft_port *np, enum nvmf_trtype trtype, } if (cmd->qid > ctrlr->num_io_queues) { mtx_unlock(&ctrlr->lock); - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); nvmft_printf(ctrlr, "attempt to create invalid I/O queue %u from %.*s\n", qid, (int)sizeof(data->hostnqn), data->hostnqn); @@ -270,7 +280,7 @@ nvmft_handoff_io_queue(struct nvmft_port *np, enum nvmf_trtype trtype, } if (ctrlr->io_qpairs[qid - 1].qp != NULL) { mtx_unlock(&ctrlr->lock); - sx_sunlock(&np->lock); + mtx_unlock(&np->lock); nvmft_printf(ctrlr, "attempt to re-create I/O queue %u from %.*s\n", qid, (int)sizeof(data->hostnqn), data->hostnqn); @@ -282,8 +292,8 @@ nvmft_handoff_io_queue(struct nvmft_port *np, enum nvmf_trtype trtype, ctrlr->io_qpairs[qid - 1].qp = qp; mtx_unlock(&ctrlr->lock); + mtx_unlock(&np->lock); nvmft_finish_accept(qp, cmd, ctrlr); - sx_sunlock(&np->lock); return (0); } @@ -382,11 +392,11 @@ nvmft_controller_terminate(void *arg, int pending) /* Remove association (CNTLID). */ np = ctrlr->np; - sx_xlock(&np->lock); + mtx_lock(&np->lock); TAILQ_REMOVE(&np->controllers, ctrlr, link); - free_unr(np->ids, ctrlr->cntlid); wakeup_np = (!np->online && TAILQ_EMPTY(&np->controllers)); - sx_xunlock(&np->lock); + mtx_unlock(&np->lock); + free_unr(np->ids, ctrlr->cntlid); if (wakeup_np) wakeup(np); diff --git a/sys/dev/nvmf/controller/nvmft_var.h b/sys/dev/nvmf/controller/nvmft_var.h index 7a1748d5999e..6d20e2c8ac11 100644 --- a/sys/dev/nvmf/controller/nvmft_var.h +++ b/sys/dev/nvmf/controller/nvmft_var.h @@ -35,7 +35,7 @@ struct nvmft_port { uint32_t max_io_qsize; bool online; - struct sx lock; + struct mtx lock; struct unrhdr *ids; TAILQ_HEAD(, nvmft_controller) controllers;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202502201540.51KFef5Y016893>