diff mbox series

dmaengine: idxd: fix missed completion on abort path

Message ID 163898288714.443911.16084982766671976640.stgit@djiang5-desk3.ch.intel.com (mailing list archive)
State Accepted
Commit 8affd8a4b5ce356c8900cfb037674f3a4a11fbdb
Headers show
Series dmaengine: idxd: fix missed completion on abort path | expand

Commit Message

Dave Jiang Dec. 8, 2021, 5:01 p.m. UTC
Ming reported that with the abort path of the descriptor submission, there
can be a window where a completed descriptor can be missed to be completed
by the irq completion thread:

CPU A				CPU B
Submit (successful)

Submit (fail)
				irq_process_work_list() // empty

llist_abort_desc()
// remove all descs from pending list

				irq_process_pending_llist() // empty
				exit idxd_wq_thread() with no processing

Add opportunistic descriptor completion in the abort path in order to
remove the missed completion.

Fixes: 6b4b87f2c31a ("dmaengine: idxd: fix submission race window")
Reported-by: Ming Li <ming4.li@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/dma/idxd/submit.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Comments

Vinod Koul Dec. 13, 2021, 5:40 a.m. UTC | #1
On 08-12-21, 10:01, Dave Jiang wrote:
> Ming reported that with the abort path of the descriptor submission, there
> can be a window where a completed descriptor can be missed to be completed
> by the irq completion thread:
> 
> CPU A				CPU B
> Submit (successful)
> 
> Submit (fail)
> 				irq_process_work_list() // empty
> 
> llist_abort_desc()
> // remove all descs from pending list
> 
> 				irq_process_pending_llist() // empty
> 				exit idxd_wq_thread() with no processing
> 
> Add opportunistic descriptor completion in the abort path in order to
> remove the missed completion.

Applied, thanks
diff mbox series

Patch

diff --git a/drivers/dma/idxd/submit.c b/drivers/dma/idxd/submit.c
index de76fb4abac2..83452fbbb168 100644
--- a/drivers/dma/idxd/submit.c
+++ b/drivers/dma/idxd/submit.c
@@ -106,6 +106,7 @@  static void llist_abort_desc(struct idxd_wq *wq, struct idxd_irq_entry *ie,
 {
 	struct idxd_desc *d, *t, *found = NULL;
 	struct llist_node *head;
+	LIST_HEAD(flist);
 
 	desc->completion->status = IDXD_COMP_DESC_ABORT;
 	/*
@@ -120,7 +121,11 @@  static void llist_abort_desc(struct idxd_wq *wq, struct idxd_irq_entry *ie,
 				found = desc;
 				continue;
 			}
-			list_add_tail(&desc->list, &ie->work_list);
+
+			if (d->completion->status)
+				list_add_tail(&d->list, &flist);
+			else
+				list_add_tail(&d->list, &ie->work_list);
 		}
 	}
 
@@ -130,6 +135,17 @@  static void llist_abort_desc(struct idxd_wq *wq, struct idxd_irq_entry *ie,
 
 	if (found)
 		complete_desc(found, IDXD_COMPLETE_ABORT);
+
+	/*
+	 * complete_desc() will return desc to allocator and the desc can be
+	 * acquired by a different process and the desc->list can be modified.
+	 * Delete desc from list so the list trasversing does not get corrupted
+	 * by the other process.
+	 */
+	list_for_each_entry_safe(d, t, &flist, list) {
+		list_del_init(&d->list);
+		complete_desc(d, IDXD_COMPLETE_NORMAL);
+	}
 }
 
 int idxd_submit_desc(struct idxd_wq *wq, struct idxd_desc *desc)