[PATCH] vfio/type1: conditional rescheduling while unpinning

Samuel Crossley posted 1 patch 13 hours ago
drivers/vfio/vfio_iommu_type1.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
[PATCH] vfio/type1: conditional rescheduling while unpinning
Posted by Samuel Crossley 13 hours ago
Tearing down a large device-passthrough DMA mapping can unpin tens to
hundreds of millions of pages in a single VFIO_IOMMU_UNMAP_DMA.
vfio_unpin_pages_remote() walks the whole contiguous range in one
uninterrupted pass with no reschedule point:

  - has_rsvd (reserved / device-memory mappings, e.g. GPU HBM/BAR mapped
    for peer DMA): a per-page put_pfn() loop, each doing a
    pfn_valid()/is_invalid_reserved_pfn() section lookup;
  - otherwise: the batched unpin_user_page_range_dirty_lock() folio walk.

The existing cond_resched() calls in the unmap path only run between
regions/chunks, so they cannot break up one giant call.

Observed on a GPU-passthrough host: unmapping a 128 GiB device-memory
region (~33.6M reserved 4K pages, has_rsvd) held a CPU 22s in the per-page
loop and tripped the soft-lockup watchdog, panicking the host:

  watchdog: BUG: soft lockup - CPU#74 stuck for 22s! [qemu-system-x86]
   put_pfn / is_invalid_reserved_pfn / pfn_valid
   vfio_unpin_pages_remote / vfio_sync_unpin / vfio_unmap_unpin
   vfio_remove_dma / vfio_iommu_type1_ioctl (VFIO_IOMMU_UNMAP_DMA)

The v6.18 batching series (d10872050ffe, d14de5b92578) optimized only the
non-reserved folio path, so it does not help this has_rsvd loop. Bound the
work per iteration and cond_resched() between chunks, mirroring the
pin-side fix in commit b1779e4f209c ("vfio/type1: conditional
rescheduling while pinning").

cond_resched() is safe on this path: the only lock held across
vfio_unpin_pages_remote() is iommu->lock, a mutex, so sleeping is allowed,
and it is taken in vfio_dma_do_unmap() and held continuously through
vfio_remove_dma() without being dropped on this path. No spinlock,
preempt-disabled, IRQ-disabled or RCU read-side section is held anywhere in
the type1 unmap path. The loop's callees (put_pfn(),
unpin_user_page_range_dirty_lock()) drop any transient folio reference and
folio lock before returning, so only the mutex is held at the reschedule
point. The path is already demonstrably sleepable: the same unmap chain
already calls cond_resched() at the region/chunk level, and the
non-reserved dirty path already takes folio_lock().

Signed-off-by: Samuel Crossley <samuelcrossley@gmail.com>
---
Signed-off-by: Sam Crossley <samuelcrossley@gmail.com>
---
 drivers/vfio/vfio_iommu_type1.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index c8151ba54de3..ce7f6051f799 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -814,22 +814,42 @@ static inline void put_valid_unreserved_pfns(unsigned long start_pfn,
 					 prot & IOMMU_WRITE);
 }
 
+/* Pages to unpin per cond_resched() when tearing down a large mapping. */
+#define VFIO_UNPIN_RESCHED_PAGES	(16UL * 1024)	/* 64MB @ 4K pages */
+
 static long vfio_unpin_pages_remote(struct vfio_dma *dma, dma_addr_t iova,
 				    unsigned long pfn, unsigned long npage,
 				    bool do_accounting)
 {
 	long unlocked = 0, locked = vpfn_pages(dma, iova, npage);
+	unsigned long remaining = npage;
 
-	if (dma->has_rsvd) {
-		unsigned long i;
+	/*
+	 * A single unmap of a very large device-passthrough mapping can unpin
+	 * hundreds of millions of pages here.  Bound the work per iteration and
+	 * cond_resched() so one VFIO_IOMMU_UNMAP_DMA cannot hold a CPU past the
+	 * soft-lockup watchdog.  Mirrors the pin-side reschedule in commit
+	 * edeca59cb88d2 ("vfio/type1: conditional rescheduling while pinning").
+	 */
+	while (remaining) {
+		unsigned long batch = min(remaining, VFIO_UNPIN_RESCHED_PAGES);
 
-		for (i = 0; i < npage; i++)
-			if (put_pfn(pfn++, dma->prot))
-				unlocked++;
-	} else {
-		put_valid_unreserved_pfns(pfn, npage, dma->prot);
-		unlocked = npage;
+		if (dma->has_rsvd) {
+			unsigned long i;
+
+			for (i = 0; i < batch; i++)
+				if (put_pfn(pfn++, dma->prot))
+					unlocked++;
+		} else {
+			put_valid_unreserved_pfns(pfn, batch, dma->prot);
+			unlocked += batch;
+			pfn += batch;
+		}
+
+		remaining -= batch;
+		cond_resched();
 	}
+
 	if (do_accounting)
 		vfio_lock_acct(dma, locked - unlocked, true);
 

---
base-commit: dc77acfeb979dded39b247b60fef0399536bfa77
change-id: 20260723-vfio-decd86bf3cf6

Best regards,
--  
Sam Crossley <samuelcrossley@gmail.com>