git: c41be266147f - main - ufshci: keep a slot for admin requests
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 03 Nov 2026 04:33:44 UTC
The branch main has been updated by jaeyoon:
URL: https://cgit.FreeBSD.org/src/commit/?id=c41be266147fd5313d4f4ccd417f95738443645a
commit c41be266147fd5313d4f4ccd417f95738443645a
Author: Jaeyoon Choi <jaeyoon@FreeBSD.org>
AuthorDate: 2026-11-03 04:17:49 +0000
Commit: Jaeyoon Choi <jaeyoon@FreeBSD.org>
CommitDate: 2026-11-03 04:29:43 +0000
ufshci: keep a slot for admin requests
The slot reserve step handed any free slot to whoever asked first, so
I/O could fill the queue and leave no slot for the driver's own admin
commands, which a reset sends through the same queue. A full queue was
also reported to CAM as busy, and CAM used up its retries on that
within microseconds.
Keep the last slot for admin requests and give CAM one opening fewer,
since letting CAM fill every I/O slot cut the throughput sharply. On a
full queue, freeze the device queue and requeue the request. Remove the
unused reserve_admin_slot hook.
Reviewed by: imp
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D61114
---
sys/dev/ufshci/ufshci_ctrlr.c | 13 ++++++++-----
sys/dev/ufshci/ufshci_private.h | 6 ++----
sys/dev/ufshci/ufshci_req_queue.c | 5 ++---
sys/dev/ufshci/ufshci_req_sdb.c | 40 +++++++++++++++++++++++++++++++--------
sys/dev/ufshci/ufshci_sim.c | 7 ++++++-
5 files changed, 50 insertions(+), 21 deletions(-)
diff --git a/sys/dev/ufshci/ufshci_ctrlr.c b/sys/dev/ufshci/ufshci_ctrlr.c
index 99b29c637e33..cd321f8bdd29 100644
--- a/sys/dev/ufshci/ufshci_ctrlr.c
+++ b/sys/dev/ufshci/ufshci_ctrlr.c
@@ -446,13 +446,16 @@ ufshci_ctrlr_construct(struct ufshci_controller *ctrlr, device_t dev)
if (error)
return (error);
- /* TODO: Separate IO and Admin slot */
-
/*
- * max_hw_pend_io is the number of slots in the transfer_req_queue.
- * Reduce num_entries by one to reserve an admin slot.
+ * Two of the slots stay out of the count handed to CAM. The
+ * reserve step keeps the last one for admin requests, so a
+ * reset's bring-up command always has somewhere to land. The one
+ * after it is headroom the queue needs to stay busy. Giving CAM
+ * as many openings as there are slots that I/O can use measured
+ * at about a seventh of the throughput, so do not raise this
+ * without testing it under load.
*/
- ctrlr->max_hw_pend_io = ctrlr->transfer_req_queue.num_entries - 1;
+ ctrlr->max_hw_pend_io = ctrlr->transfer_req_queue.num_entries - 2;
/* Create a thread for the taskqueue. */
ctrlr->taskqueue = taskqueue_create("ufshci_taskq", M_WAITOK,
diff --git a/sys/dev/ufshci/ufshci_private.h b/sys/dev/ufshci/ufshci_private.h
index 32818ca0392e..b268e337976e 100644
--- a/sys/dev/ufshci/ufshci_private.h
+++ b/sys/dev/ufshci/ufshci_private.h
@@ -129,9 +129,7 @@ struct ufshci_qops {
void (*disable)(struct ufshci_controller *ctrlr,
struct ufshci_req_queue *req_queue);
int (*reserve_slot)(struct ufshci_req_queue *req_queue,
- struct ufshci_tracker **tr);
- int (*reserve_admin_slot)(struct ufshci_req_queue *req_queue,
- struct ufshci_tracker **tr);
+ struct ufshci_tracker **tr, bool admin);
void (*ring_doorbell)(struct ufshci_controller *ctrlr,
struct ufshci_tracker *tr);
bool (*is_completed)(struct ufshci_controller *ctrlr,
@@ -532,7 +530,7 @@ void ufshci_req_sdb_disable(struct ufshci_controller *ctrlr,
int ufshci_req_sdb_enable(struct ufshci_controller *ctrlr,
struct ufshci_req_queue *req_queue);
int ufshci_req_sdb_reserve_slot(struct ufshci_req_queue *req_queue,
- struct ufshci_tracker **tr);
+ struct ufshci_tracker **tr, bool admin);
void ufshci_req_sdb_utmr_ring_doorbell(struct ufshci_controller *ctrlr,
struct ufshci_tracker *tr);
void ufshci_req_sdb_utr_ring_doorbell(struct ufshci_controller *ctrlr,
diff --git a/sys/dev/ufshci/ufshci_req_queue.c b/sys/dev/ufshci/ufshci_req_queue.c
index 96de50abd153..ee1ce97e6942 100644
--- a/sys/dev/ufshci/ufshci_req_queue.c
+++ b/sys/dev/ufshci/ufshci_req_queue.c
@@ -26,7 +26,6 @@ static const struct ufshci_qops sdb_utmr_qops = {
.enable = ufshci_req_sdb_enable,
.disable = ufshci_req_sdb_disable,
.reserve_slot = ufshci_req_sdb_reserve_slot,
- .reserve_admin_slot = ufshci_req_sdb_reserve_slot,
.ring_doorbell = ufshci_req_sdb_utmr_ring_doorbell,
.is_completed = ufshci_req_sdb_utmr_is_completed,
.clear_cpl_ntf = ufshci_req_sdb_utmr_clear_cpl_ntf,
@@ -41,7 +40,6 @@ static const struct ufshci_qops sdb_utr_qops = {
.enable = ufshci_req_sdb_enable,
.disable = ufshci_req_sdb_disable,
.reserve_slot = ufshci_req_sdb_reserve_slot,
- .reserve_admin_slot = ufshci_req_sdb_reserve_slot,
.ring_doorbell = ufshci_req_sdb_utr_ring_doorbell,
.is_completed = ufshci_req_sdb_utr_is_completed,
.clear_cpl_ntf = ufshci_req_sdb_utr_clear_cpl_ntf,
@@ -805,7 +803,8 @@ _ufshci_req_queue_submit_request(struct ufshci_req_queue *req_queue,
if (req_queue->ctrlr->is_failed)
return (ENXIO);
- error = req_queue->qops.reserve_slot(req_queue, &tr);
+ error = req_queue->qops.reserve_slot(req_queue, &tr,
+ req->is_admin);
if (error != 0) {
ufshci_printf(req_queue->ctrlr, "Failed to get tracker");
return (error);
diff --git a/sys/dev/ufshci/ufshci_req_sdb.c b/sys/dev/ufshci/ufshci_req_sdb.c
index 6e756c523830..ed062c95bca6 100644
--- a/sys/dev/ufshci/ufshci_req_sdb.c
+++ b/sys/dev/ufshci/ufshci_req_sdb.c
@@ -488,20 +488,44 @@ out:
return (error);
}
+static bool
+ufshci_req_sdb_take_slot(struct ufshci_hw_queue *hwq, int slot,
+ struct ufshci_tracker **tr)
+{
+ mtx_assert(&hwq->qlock, MA_OWNED);
+
+ if (hwq->act_tr[slot]->slot_state != UFSHCI_SLOT_STATE_FREE)
+ return (false);
+
+ *tr = hwq->act_tr[slot];
+ (*tr)->hwq = hwq;
+ return (true);
+}
+
int
ufshci_req_sdb_reserve_slot(struct ufshci_req_queue *req_queue,
- struct ufshci_tracker **tr)
+ struct ufshci_tracker **tr, bool admin)
{
struct ufshci_hw_queue *hwq = &req_queue->hwq[UFSHCI_SDB_Q];
- uint8_t i;
+ int last = hwq->num_trackers - 1;
+ int slot;
- for (i = 0; i < req_queue->num_entries; i++) {
- if (hwq->act_tr[i]->slot_state == UFSHCI_SLOT_STATE_FREE) {
- *tr = hwq->act_tr[i];
- (*tr)->hwq = hwq;
- return (0);
- }
+ mtx_assert(&hwq->qlock, MA_OWNED);
+
+ /*
+ * The last slot is held for admin requests, which a reset needs
+ * for its bring-up. Admin requests search down so they take it.
+ */
+ if (admin || req_queue->is_task_mgmt) {
+ for (slot = last; slot >= 0; slot--)
+ if (ufshci_req_sdb_take_slot(hwq, slot, tr))
+ return (0);
+ } else {
+ for (slot = 0; slot < last; slot++)
+ if (ufshci_req_sdb_take_slot(hwq, slot, tr))
+ return (0);
}
+
return (EBUSY);
}
diff --git a/sys/dev/ufshci/ufshci_sim.c b/sys/dev/ufshci/ufshci_sim.c
index dc5198ae6f35..a6e160a99083 100644
--- a/sys/dev/ufshci/ufshci_sim.c
+++ b/sys/dev/ufshci/ufshci_sim.c
@@ -200,7 +200,12 @@ ufshchi_sim_scsiio(struct cam_sim *sim, union ccb *ccb)
ccb->ccb_h.status |= CAM_SIM_QUEUED;
error = ufshci_ctrlr_submit_transfer_request(ctrlr, req);
if (error == EBUSY) {
- ccb->ccb_h.status = CAM_SCSI_BUSY;
+ /*
+ * No slot is free. Freeze and requeue so CAM neither
+ * spins nor burns its retries.
+ */
+ xpt_freeze_devq(ccb->ccb_h.path, 1);
+ ccb->ccb_h.status = CAM_REQUEUE_REQ | CAM_DEV_QFRZN;
ufshci_free_request(req);
xpt_done(ccb);
return;