git: 088c6fc1f9bf - main - ufshci: claim a slot before completing it

The branch main has been updated by jaeyoon:

URL: https://cgit.FreeBSD.org/src/commit/?id=088c6fc1f9bf938fcad465fe7899d64cf4453b69

commit 088c6fc1f9bf938fcad465fe7899d64cf4453b69
Author:     Jaeyoon Choi <jaeyoon@FreeBSD.org>
AuthorDate: 2026-11-03 04:16:36 +0000
Commit:     Jaeyoon Choi <jaeyoon@FreeBSD.org>
CommitDate: 2026-11-03 04:29:42 +0000

    ufshci: claim a slot before completing it
    
    A request is completed with the queue lock dropped, because the
    completion callback may submit the next request and take that lock
    again. The completion scan checked a slot under the lock, dropped the
    lock, and only then completed the request, with the slot still marked
    scheduled. The failure path and the abort completion look for such
    slots under the same lock, so one of them could pick the same slot in
    that window. The request was then completed twice, freed twice, and
    its CCB went back to CAM twice. After a reset every in-flight slot
    reads as done at once, so the window was easy to hit.
    
    No existing state could mark that window. FREE would hand the slot to
    a new request while the old completion still reads it, RESERVED means
    a submit in progress, and NEED_ERROR_HANDLING is about to mean a
    request that a reset has claimed but nobody has completed yet.
    
    Add a completing state. Every path that completes a slot sets it
    under the lock before dropping the lock, and every path skips a slot
    in that state. The abort completion now takes only a slot that is
    still scheduled, where it used to take anything that was not free.
    
    Reviewed by:            imp
    Sponsored by:           Samsung Electronics
    Differential Revision:  https://reviews.freebsd.org/D61113
---
 sys/dev/ufshci/ufshci_private.h   |  1 +
 sys/dev/ufshci/ufshci_req_queue.c | 17 +++++++++++------
 sys/dev/ufshci/ufshci_req_sdb.c   |  8 +++++++-
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/sys/dev/ufshci/ufshci_private.h b/sys/dev/ufshci/ufshci_private.h
index 872518ee30b3..32818ca0392e 100644
--- a/sys/dev/ufshci/ufshci_private.h
+++ b/sys/dev/ufshci/ufshci_private.h
@@ -82,6 +82,7 @@ enum ufshci_slot_state {
 	UFSHCI_SLOT_STATE_SCHEDULED = 0x2,
 	UFSHCI_SLOT_STATE_TIMEOUT = 0x3,
 	UFSHCI_SLOT_STATE_NEED_ERROR_HANDLING = 0x4,
+	UFSHCI_SLOT_STATE_COMPLETING = 0x5,
 };
 
 struct ufshci_tracker {
diff --git a/sys/dev/ufshci/ufshci_req_queue.c b/sys/dev/ufshci/ufshci_req_queue.c
index ee73a0ca5462..96de50abd153 100644
--- a/sys/dev/ufshci/ufshci_req_queue.c
+++ b/sys/dev/ufshci/ufshci_req_queue.c
@@ -209,7 +209,7 @@ ufshci_req_queue_fail(struct ufshci_controller *ctrlr,
 
 	mtx_lock(&hwq->qlock);
 
-	for (i = 0; i < req_queue->num_trackers; i++) {
+	for (i = 0; i < hwq->num_trackers; i++) {
 		tr = hwq->act_tr[i];
 
 		/*
@@ -222,11 +222,11 @@ ufshci_req_queue_fail(struct ufshci_controller *ctrlr,
 			continue;
 
 		/*
-		 * Claim the tracker under the lock. The completion
-		 * scan only completes SCHEDULED slots, so it will
-		 * skip this one while the lock is dropped.
+		 * Claim the tracker under the lock. Every other walker
+		 * skips a slot in this state, so the tracker cannot
+		 * complete twice while the lock is dropped.
 		 */
-		tr->slot_state = UFSHCI_SLOT_STATE_NEED_ERROR_HANDLING;
+		tr->slot_state = UFSHCI_SLOT_STATE_COMPLETING;
 		mtx_unlock(&hwq->qlock);
 		ufshci_req_queue_manual_complete_tracker(tr,
 		    UFSHCI_DESC_ABORTED,
@@ -491,7 +491,12 @@ ufshci_abort_complete(void *arg, const struct ufshci_completion *status,
 	 * complete the command manually.
 	 */
 	mtx_lock(&tr->hwq->qlock);
-	if (tr->slot_state != UFSHCI_SLOT_STATE_FREE) {
+	/*
+	 * Only a scheduled slot still waits on the controller. Claim it
+	 * before the unlock, so no other path completes it as well.
+	 */
+	if (tr->slot_state == UFSHCI_SLOT_STATE_SCHEDULED) {
+		tr->slot_state = UFSHCI_SLOT_STATE_COMPLETING;
 		mtx_unlock(&tr->hwq->qlock);
 		/*
 		 * An I/O has timed out, and the controller was unable to abort
diff --git a/sys/dev/ufshci/ufshci_req_sdb.c b/sys/dev/ufshci/ufshci_req_sdb.c
index 2404354bfade..6e756c523830 100644
--- a/sys/dev/ufshci/ufshci_req_sdb.c
+++ b/sys/dev/ufshci/ufshci_req_sdb.c
@@ -595,7 +595,7 @@ ufshci_req_sdb_process_cpl(struct ufshci_req_queue *req_queue)
 	bus_dmamap_sync(hwq->dma_tag_queue, hwq->queuemem_map,
 	    BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
 
-	for (slot = 0; slot < req_queue->num_entries; slot++) {
+	for (slot = 0; slot < hwq->num_trackers; slot++) {
 		bool completed;
 
 		tr = hwq->act_tr[slot];
@@ -609,6 +609,12 @@ ufshci_req_sdb_process_cpl(struct ufshci_req_queue *req_queue)
 		mtx_lock(&hwq->qlock);
 		completed = tr->slot_state == UFSHCI_SLOT_STATE_SCHEDULED &&
 		    req_queue->qops.is_completed(req_queue->ctrlr, slot);
+		/*
+		 * Claim the slot while the lock is held, so no other path
+		 * completes this request as well.
+		 */
+		if (completed)
+			tr->slot_state = UFSHCI_SLOT_STATE_COMPLETING;
 		mtx_unlock(&hwq->qlock);
 		if (completed) {
 			ufshci_req_queue_complete_tracker(tr);