git: 97ca2ada80b8 - main - nvmft: Switch the per-port lock from sx(9) to mtx(9)
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 20 Feb 2025 15:40:41 UTC
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;