On Wed, Jul 15, 2026 at 11:16:52AM -0700, Stanislav Kinsburskii wrote: > Several GPU SVM paths take mmap_read_lock() only to call hmm_range_fault() > and open-code mmu interval sequence setup before each HMM walk. They also > retry -EBUSY until HMM_RANGE_DEFAULT_TIMEOUT expires. > > Use hmm_range_fault_unlocked_timeout() for those faults. The HMM helper now > owns mmap_lock acquisition and refreshes range->notifier_seq for its > internal retries, while GPU SVM keeps its existing driver-lock validation > with mmu_interval_read_retry() after a successful fault. > > Pass HMM_RANGE_DEFAULT_TIMEOUT as the helper retry budget for each HMM > fault attempt. This scopes the timeout to repeated HMM notifier retries > while preserving the outer retry loops that restart when the interval is > invalidated before GPU SVM updates or consumes the mapping state. >
This part doesn't seem right for get_pages(), see below. > Leave drm_gpusvm_check_pages() on hmm_range_fault() because that path is > called with the mmap lock already held by its caller. > > Reviewed-by: Jason Gunthorpe <[email protected]> > Signed-off-by: Stanislav Kinsburskii <[email protected]> > --- > drivers/gpu/drm/drm_gpusvm.c | 61 > +++++------------------------------------- > 1 file changed, 7 insertions(+), 54 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c > index 958cb605aedd..de5bbfe58ee9 100644 > --- a/drivers/gpu/drm/drm_gpusvm.c > +++ b/drivers/gpu/drm/drm_gpusvm.c > @@ -773,8 +773,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); > @@ -788,22 +787,7 @@ enum drm_gpusvm_scan_result drm_gpusvm_scan_mm(struct > drm_gpusvm_range *range, > hmm_range.hmm_pfns = pfns; > > retry: > - hmm_range.notifier_seq = mmu_interval_read_begin(notifier); > - mmap_read_lock(range->gpusvm->mm); > - > - while (true) { > - err = hmm_range_fault(&hmm_range); > - if (err == -EBUSY) { > - if (time_after(jiffies, timeout)) > - break; > - > - hmm_range.notifier_seq = > - mmu_interval_read_begin(notifier); > - continue; > - } > - break; > - } > - mmap_read_unlock(range->gpusvm->mm); > + err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout); > if (err) > goto err_free; > > @@ -1406,8 +1390,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; > @@ -1422,9 +1405,6 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > struct dma_iova_state *state = &svm_pages->state; > > retry: > - if (time_after(jiffies, timeout)) > - return -EBUSY; > - I think that by deleting the code above, you have changed this function's semantics by removing the hard cap of HMM_RANGE_DEFAULT_TIMEOUT. This code was added because, on some non-production platforms, the timing in this function could cause it to livelock. Is there any reason this was remove aside from timeout variable not being a deadline now? You likely should add the deadline back in. Matt > hmm_range.notifier_seq = mmu_interval_read_begin(notifier); > if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages)) > goto set_seqno; > @@ -1439,21 +1419,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > } > > hmm_range.hmm_pfns = pfns; > - while (true) { > - mmap_read_lock(mm); > - err = hmm_range_fault(&hmm_range); > - mmap_read_unlock(mm); > - > - if (err == -EBUSY) { > - if (time_after(jiffies, timeout)) > - break; > - > - hmm_range.notifier_seq = > - mmu_interval_read_begin(notifier); > - continue; > - } > - break; > - } > + err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout); > mmput(mm); > if (err) > goto err_free; > @@ -1720,8 +1686,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)); > @@ -1736,24 +1701,12 @@ int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm, > return -ENOMEM; > > hmm_range.hmm_pfns = pfns; > - while (!time_after(jiffies, timeout)) { > - hmm_range.notifier_seq = mmu_interval_read_begin(notifier); > - if (time_after(jiffies, timeout)) { > - err = -ETIME; > - break; > - } > - > - mmap_read_lock(mm); > - err = hmm_range_fault(&hmm_range); > - mmap_read_unlock(mm); > - if (err != -EBUSY) > - break; > - } > + err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout); > > kvfree(pfns); > mmput(mm); > > - return err; > + return err == -EBUSY ? -ETIME : err; > } > EXPORT_SYMBOL_GPL(drm_gpusvm_range_evict); > > >
