[PATCH v2 0/4] mm/hmm: Clarify notifier retry state and scope HMM timeouts

[PATCH v2 0/4] mm/hmm: Clarify notifier retry state and scope HMM timeouts

[Stanislav Kinsburskii]

This small fixup series applies on top of:

  [PATCH v8 0/8] mm/hmm: Add mmap lock-drop support for userfaultfd-backed mappings

The first patch updates the HMM documentation example to make the
mmu_interval_read_retry() state explicit: callers should use the notifier and
notifier_seq stored in the same hmm_range that was passed to
hmm_range_fault_unlocked_timeout().

The remaining patches adjust nouveau, amdxdna, and drm_gpusvm users so the
timeout passed to hmm_range_fault_unlocked_timeout() is treated as a relative
HMM retry budget. These callers no longer keep an absolute deadline around
their outer driver retry loops or pass a computed remaining time into HMM.

This keeps the timeout scoped to HMM's internal mmu-notifier retry handling. If
HMM succeeds and the driver later observes an invalidation through
mmu_interval_read_retry(), the driver retries the operation with a fresh HMM
retry budget.

Changes in v2:
  - Kept the nouveau outer absolute timeout around the
    mmu_interval_read_retry() loop. hmm_range_fault_unlocked_timeout() only
    bounds HMM’s internal retries, while nouveau faults are handled from a GPU
    fault worker, so userspace fatal signals cannot break an endless stream of
    invalidations there.
  - Updated nouveau to use time_after_eq() before calling HMM, so the remaining
    timeout passed to hmm_range_fault_unlocked_timeout() is always positive and
    never 0, which would mean retry indefinitely.
  - Updated the nouveau fixup commit message to explain the worker-thread
    timeout issue and the time_after_eq() boundary behavior.
  - Fixed the amdxdna fixup commit message. It now describes
    aie2_populate_range() correctly instead of carrying stale nouveau prose,
    and notes that command submission still keeps its broader timeout while HMM
    gets a fresh relative retry budget.


---

Stanislav Kinsburskii (4):
      fixup! mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support
      fixup! drm/nouveau: use hmm_range_fault_unlocked_timeout() for SVM faults
      fixup! accel/amdxdna: use hmm_range_fault_unlocked_timeout() for range population
      fixup! drm/gpusvm: use hmm_range_fault_unlocked_timeout() for range faults


 Documentation/mm/hmm.rst              |    5 +++--
 drivers/accel/amdxdna/aie2_ctx.c      |   12 ++++--------
 drivers/gpu/drm/drm_gpusvm.c          |   21 ++++++---------------
 drivers/gpu/drm/nouveau/nouveau_svm.c |   14 ++++++++++----
 4 files changed, 23 insertions(+), 29 deletions(-)

[PATCH v2 1/4] fixup! mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support

[Stanislav Kinsburskii]

The hmm_range_fault_unlocked_timeout() example checks for a concurrent
mmu_interval_notifier invalidation after the HMM fault has succeeded.
The sequence number used for that check is stored in range->notifier_seq
by hmm_range_fault_unlocked_timeout(), so the retry check should use the
same notifier stored in range as well.

Update the example to pass range.notifier to mmu_interval_read_retry(),
and spell out that the retry check uses the notifier and sequence number
stored in range by the HMM fault helper. This makes the relationship
between the HMM walk and the later invalidation check explicit.

Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
---
 Documentation/mm/hmm.rst |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/mm/hmm.rst b/Documentation/mm/hmm.rst
index 5c88d2cf0818..fc1b8dc19825 100644
--- a/Documentation/mm/hmm.rst
+++ b/Documentation/mm/hmm.rst
@@ -206,7 +206,7 @@ The usage pattern is::
           goto out_put;
 
       take_lock(driver->update);
-      if (mmu_interval_read_retry(&interval_sub, range.notifier_seq)) {
+      if (mmu_interval_read_retry(range.notifier, range.notifier_seq)) {
           release_lock(driver->update);
           goto again;
       }
@@ -225,7 +225,8 @@ The usage pattern is::
 The driver->update lock is the same lock that the driver takes inside its
 invalidate() callback. That lock must be held before calling
 mmu_interval_read_retry() to avoid any race with a concurrent CPU page table
-update.
+update. The retry check must use the same notifier and sequence number stored
+in ``range`` by ``hmm_range_fault_unlocked_timeout()``.
 
 Holding the mmap lock across HMM faults
 =======================================

[PATCH v2 2/4] fixup! drm/nouveau: use hmm_range_fault_unlocked_timeout() for SVM faults

[Stanislav Kinsburskii]

nouveau_range_fault() now uses hmm_range_fault_unlocked_timeout() for
the HMM fault path. Pass the remaining SVM fault timeout into HMM as a
positive relative timeout instead of open-coding the HMM retry loop in
nouveau.

Keep the outer absolute timeout around nouveau's
mmu_interval_read_retry() loop. HMM's timeout only bounds retries while
HMM is walking and faulting the range. Once HMM returns successfully, a
continuous stream of mmu-notifier invalidations can still make
mmu_interval_read_retry() restart the operation. This path runs from the
GPU fault worker, so fatal signals for the faulting userspace task cannot
be relied on to break that outer loop.

Use time_after_eq() before the HMM call so the remaining timeout passed
to HMM is always positive. This preserves the old timeout behavior at the
expiry boundary and avoids passing 0, which means retry indefinitely, to
hmm_range_fault_unlocked_timeout().

Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_svm.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 4cfb6eb7c771..58735446d783 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -678,14 +678,20 @@ static int nouveau_range_fault(struct nouveau_svmm *svmm,
 	range.end = notifier->notifier.interval_tree.last + 1;
 
 	while (true) {
-		if (time_after(jiffies, timeout)) {
+		long remaining = timeout - jiffies;
+
+		/*
+		 * The HMM timeout only bounds retries while HMM is walking and
+		 * faulting the range. This fault is handled by a kernel worker,
+		 * so fatal signals from the faulting process cannot stop an
+		 * endless stream of invalidations here.
+		 */
+		if (time_after_eq(jiffies, timeout)) {
 			ret = -EBUSY;
 			goto out;
 		}
 
-		ret = hmm_range_fault_unlocked_timeout(&range,
-						       max(timeout - jiffies,
-							   1L));
+		ret = hmm_range_fault_unlocked_timeout(&range, remaining);
 		if (ret)
 			goto out;
 

[PATCH v2 3/4] fixup! accel/amdxdna: use hmm_range_fault_unlocked_timeout() for range population

[Stanislav Kinsburskii]

aie2_populate_range() now uses hmm_range_fault_unlocked_timeout() to let
HMM own mmap_lock acquisition and retry handling while populating an
invalid user mapping. The timeout passed to that helper is a relative
HMM retry budget, not an absolute deadline.

Pass HMM_RANGE_DEFAULT_TIMEOUT directly to the HMM helper instead of
computing the remaining time from a local absolute deadline. The broader
command submission path still keeps its existing timeout around repeated
range population attempts, while each HMM fault attempt receives a fresh
retry budget for HMM's internal mmu-notifier retries.

Keep the existing driver-visible timeout errno by translating -EBUSY
from hmm_range_fault_unlocked_timeout() to -ETIME on return.

Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
---
 drivers/accel/amdxdna/aie2_ctx.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
index 548ba4315554..21f2817751f9 100644
--- a/drivers/accel/amdxdna/aie2_ctx.c
+++ b/drivers/accel/amdxdna/aie2_ctx.c
@@ -1037,7 +1037,7 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
 	bool found;
 	int ret;
 
-	timeout = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+	timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
 again:
 	found = false;
 	down_write(&xdna->notifier_lock);
@@ -1062,13 +1062,9 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
 		return -EFAULT;
 	}
 
-	ret = hmm_range_fault_unlocked_timeout(&mapp->range,
-			max_t(long, timeout - jiffies, 1));
-	if (ret) {
-		if (ret == -EBUSY)
-			ret = -ETIME;
+	ret = hmm_range_fault_unlocked_timeout(&mapp->range, timeout);
+	if (ret)
 		goto put_mm;
-	}
 
 	down_write(&xdna->notifier_lock);
 	if (mmu_interval_read_retry(&mapp->notifier, mapp->range.notifier_seq)) {
@@ -1086,7 +1082,7 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
 put_mm:
 	amdxdna_umap_put(mapp);
 	mmput(mm);
-	return ret;
+	return ret == -EBUSY ? -ETIME : ret;
 }
 
 int aie2_cmd_submit(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job, u64 *seq)

[PATCH v2 4/4] fixup! drm/gpusvm: use hmm_range_fault_unlocked_timeout() for range faults

[Stanislav Kinsburskii]

The timeout passed to hmm_range_fault_unlocked_timeout() is a relative
retry budget for HMM's internal mmu-notifier retry loop. drm_gpusvm was
still keeping an absolute deadline around the outer driver retry logic
and passing the remaining time into HMM.

Pass HMM_RANGE_DEFAULT_TIMEOUT directly to
hmm_range_fault_unlocked_timeout() on each HMM fault attempt instead.
If HMM succeeds but the later drm_gpusvm-side mmu_interval_read_retry()
check observes an invalidation, retry with a fresh HMM retry budget.

This keeps the timeout focused on repeated notifier retries inside HMM,
while avoiding an outer deadline that also accounts unrelated driver-side
work after HMM has made progress.

Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
---
 drivers/gpu/drm/drm_gpusvm.c |   21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index b8f2dd9982f5..76e8a0028c7f 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -852,8 +852,7 @@ enum drm_gpusvm_scan_result drm_gpusvm_scan_mm(struct drm_gpusvm_range *range,
 		.end = end,
 		.dev_private_owner = dev_private_owner,
 	};
-	unsigned long timeout =
-		jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+	unsigned long timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
 	enum drm_gpusvm_scan_result state = DRM_GPUSVM_SCAN_UNPOPULATED, new_state;
 	unsigned long *pfns;
 	unsigned long npages = npages_in_range(start, end);
@@ -867,8 +866,7 @@ enum drm_gpusvm_scan_result drm_gpusvm_scan_mm(struct drm_gpusvm_range *range,
 	hmm_range.hmm_pfns = pfns;
 
 retry:
-	err = hmm_range_fault_unlocked_timeout(&hmm_range,
-					       max(timeout - jiffies, 1L));
+	err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout);
 	if (err)
 		goto err_free;
 
@@ -1459,8 +1457,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
 		.dev_private_owner = ctx->device_private_page_owner,
 	};
 	void *zdd;
-	unsigned long timeout =
-		jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+	unsigned long timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
 	unsigned long i, j;
 	unsigned long npages = npages_in_range(pages_start, pages_end);
 	unsigned long num_dma_mapped;
@@ -1478,9 +1475,6 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
 		return -EINVAL;
 
 retry:
-	if (time_after(jiffies, timeout))
-		return -EBUSY;
-
 	hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
 	if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages))
 		goto set_seqno;
@@ -1495,8 +1489,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
 	}
 
 	hmm_range.hmm_pfns = pfns;
-	err = hmm_range_fault_unlocked_timeout(&hmm_range,
-				max_t(long, timeout - jiffies, 1));
+	err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout);
 	mmput(mm);
 	if (err)
 		goto err_free;
@@ -1718,8 +1711,7 @@ int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm,
 		.end = drm_gpusvm_range_end(range),
 		.dev_private_owner = NULL,
 	};
-	unsigned long timeout =
-		jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+	unsigned long timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
 	unsigned long *pfns;
 	unsigned long npages = npages_in_range(drm_gpusvm_range_start(range),
 					       drm_gpusvm_range_end(range));
@@ -1734,8 +1726,7 @@ int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm,
 		return -ENOMEM;
 
 	hmm_range.hmm_pfns = pfns;
-	err = hmm_range_fault_unlocked_timeout(&hmm_range,
-				max_t(long, timeout - jiffies, 1));
+	err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout);
 
 	kvfree(pfns);
 	mmput(mm);

Re: [PATCH v2 3/4] fixup! accel/amdxdna: use hmm_range_fault_unlocked_timeout() for range population

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Livelock in aie2_populate_range() retry loop due to lack of absolute timeout.
- [High] Data race when concurrently populating shared BO mappings.
--

--- Patch [3]: [PATCH v2 3/4] fixup! accel/amdxdna: use hmm_range_fault_unlocked_timeout() for range population ---
commit 0ad43904da8ddad180e9ceac49d9b0988660e0ec
Author: Stanislav Kinsburskii <skinsburskii@gmail.com>

fixup! accel/amdxdna: use hmm_range_fault_unlocked_timeout() for range population
    
aie2_populate_range() now uses hmm_range_fault_unlocked_timeout() to let
HMM own mmap_lock acquisition and retry handling while populating an
invalid user mapping. The timeout passed to that helper is a relative
HMM retry budget, not an absolute deadline.

> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
> index 548ba4315554f..21f2817751f9e 100644
> --- a/drivers/accel/amdxdna/aie2_ctx.c
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
> @@ -1062,13 +1062,9 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
>  		return -EFAULT;
>  	}
>  
> -	ret = hmm_range_fault_unlocked_timeout(&mapp->range,
> -			max_t(long, timeout - jiffies, 1));
> -	if (ret) {
> -		if (ret == -EBUSY)
> -			ret = -ETIME;
> +	ret = hmm_range_fault_unlocked_timeout(&mapp->range, timeout);

[Severity: High]
This is a pre-existing issue, but is there a data race when concurrently
populating shared BO mappings?

In aie2_cmd_submit(), the job submission explicitly drops reservations and
locks before calling aie2_populate_range():

drivers/accel/amdxdna/aie2_ctx.c:aie2_cmd_submit() {
    ...
    if (abo->mem.map_invalid) {
        up_read(&xdna->notifier_lock);
        drm_gem_unlock_reservations(job->bos, job->bo_cnt, &acquire_ctx);
        ...
        ret = aie2_populate_range(abo);
    ...
}

If two threads concurrently submit jobs using a shared buffer object, they
could both see abo->mem.map_invalid as true, drop their locks, and
simultaneously call aie2_populate_range() for the same BO.

Could this cause concurrent uncoordinated modifications to the same
mapp->range structure, causing a data race on range->notifier_seq and
concurrent writes to the range->hmm_pfns array during the call to
hmm_range_fault_unlocked_timeout() here?

> +	if (ret)
>  		goto put_mm;
> -	}
>  
>  	down_write(&xdna->notifier_lock);
>  	if (mmu_interval_read_retry(&mapp->notifier, mapp->range.notifier_seq)) {

[Severity: High]
This is also a pre-existing issue, but does this create a livelock if an
unprivileged user repeatedly invalidates a memory range?

The code executes a 'goto again' if mmu_interval_read_retry() returns true,
passing a fresh timeout budget to hmm_range_fault_unlocked_timeout() on every
iteration.

Because there is no overarching absolute deadline within this loop, can the
thread spin indefinitely here?

The outer absolute timeout in aie2_cmd_submit() wouldn't provide protection
because aie2_populate_range() never returns while spinning.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178406760622.1106335.2379450382728057793.stgit@skinsburskii?part=3

Re: [PATCH v2 1/4] fixup! mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] hmm_range_fault_locked() performs VMA iteration on an external mm_struct without verifying if the address space is stable, risking a crash on an unstable maple tree.
--

--- Patch [1]: [PATCH v2 1/4] fixup! mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support ---
commit a32e804e265d5dd30e11762ac51a889fc0abfebc
Author: Stanislav Kinsburskii <skinsburskii@gmail.com>

fixup! mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support

This commit updates the example in the HMM documentation to pass the notifier
stored in range to mmu_interval_read_retry(). It explicitly details the
relationship between the HMM walk and the invalidation check.

> diff --git a/Documentation/mm/hmm.rst b/Documentation/mm/hmm.rst
> index 5c88d2cf08183..fc1b8dc198250 100644
> --- a/Documentation/mm/hmm.rst
> +++ b/Documentation/mm/hmm.rst
> @@ -206,7 +206,7 @@ The usage pattern is::
>            goto out_put;
>  
>        take_lock(driver->update);
> -      if (mmu_interval_read_retry(&interval_sub, range.notifier_seq)) {
> +      if (mmu_interval_read_retry(range.notifier, range.notifier_seq)) {
>            release_lock(driver->update);
>            goto again;
>        }

[ ... ]

> @@ -225,7 +225,8 @@ The usage pattern is::
>  The driver->update lock is the same lock that the driver takes inside its
>  invalidate() callback. That lock must be held before calling
>  mmu_interval_read_retry() to avoid any race with a concurrent CPU page table
> -update.
> +update. The retry check must use the same notifier and sequence number stored
> +in ``range`` by ``hmm_range_fault_unlocked_timeout()``.
>  
>  Holding the mmap lock across HMM faults
>  =======================================

[Severity: High]
This is a pre-existing issue, but while reviewing the documentation for
HMM faults, I noticed hmm_range_fault_locked() in mm/hmm.c might be missing
a check for a stable address space before traversing VMAs:

mm/hmm.c:hmm_range_fault_locked() {
    ...
	mmap_assert_locked(mm);

	do {
		/* If range is no longer valid force retry. */
		if (mmu_interval_check_retry(range->notifier,
					     range->notifier_seq))
			return -EBUSY;
		ret = walk_page_range(mm, hmm_vma_walk.last, range->end,
				      &hmm_walk_ops, &hmm_vma_walk);
    ...
}

Does this path need to call check_stable_address_space(mm) before iterating
the VMAs with walk_page_range()?

If the mm becomes unstable (for instance, if an OOM reaper sets MMF_UNSTABLE
and modifies the maple tree concurrently while a device driver processes a
page fault for a user process), could iterating the maple tree on an
external mm_struct encounter XA_ZERO_ENTRY markers and lead to undefined
behavior or a panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178406760622.1106335.2379450382728057793.stgit@skinsburskii?part=1

Re: [PATCH v2 4/4] fixup! drm/gpusvm: use hmm_range_fault_unlocked_timeout() for range faults

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing mmput() on memory allocation failure causes mm_struct leak.
--

--- Patch [4]: [PATCH v2 4/4] fixup! drm/gpusvm: use hmm_range_fault_unlocked_timeout() for range faults ---
commit ece4b74fbf695a6f71fbfd09dfe6eb481688809b
Author: Stanislav Kinsburskii <skinsburskii@gmail.com>

fixup! drm/gpusvm: use hmm_range_fault_unlocked_timeout() for range faults

This commit updates the timeout handling for hmm_range_fault_unlocked_timeout()
by passing the HMM_RANGE_DEFAULT_TIMEOUT directly instead of calculating an
absolute deadline. This keeps the timeout focused on repeated notifier retries
inside HMM.

> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index b8f2dd9982f53..76e8a0028c7fc 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
[ ... ]
> @@ -1734,8 +1726,7 @@ int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm,
>  		return -ENOMEM;

[Severity: High]
This is a pre-existing issue, but does this error path leak the mm_struct
reference?

Looking at drm_gpusvm_range_evict(), if kvmalloc_array() fails to allocate
the pfns array, it returns -ENOMEM directly without dropping the reference
acquired earlier via mmget_not_zero():

int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm,
                           struct drm_gpusvm_range *range)
{
        ...
        if (!mmget_not_zero(mm))
                return -EFAULT;

        pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
        if (!pfns)
                return -ENOMEM;
        ...
}

Can this permanently pin the process address space if an unprivileged
userspace application creates a massive GPU SVM range that causes the
allocation to fail?

>  
>  	hmm_range.hmm_pfns = pfns;
> -	err = hmm_range_fault_unlocked_timeout(&hmm_range,
> -				max_t(long, timeout - jiffies, 1));
> +	err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout);
>  
>  	kvfree(pfns);
>  	mmput(mm);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178406760622.1106335.2379450382728057793.stgit@skinsburskii?part=4