On Mon, Jul 13, 2026 at 09:30:35AM -0700, Matthew Brost wrote: > On Fri, Jul 10, 2026 at 02:27:19PM -0700, Stanislav Kinsburskii wrote: > > Please send series like this to [email protected] list too > as this will trigger our CI which expercises the change paths changed in > this series. > > > Several GPU SVM paths take mmap_read_lock() only to call hmm_range_fault(), > > then retry -EBUSY until HMM_RANGE_DEFAULT_TIMEOUT expires. Those paths use > > MMU interval notifiers whose mm matches the mm that was locked for the HMM > > fault. > > > > Use hmm_range_fault_unlocked_timeout() for those faults and pass the > > remaining retry budget to HMM. The helper owns mmap_lock acquisition and > > refreshes range->notifier_seq internally for each retry, while GPU SVM > > keeps its existing driver-lock validation with mmu_interval_read_retry() > > after a successful fault. > > > > Leave drm_gpusvm_check_pages() on hmm_range_fault() because that path is > > called with the mmap lock already held by its caller. > > > > Signed-off-by: Stanislav Kinsburskii <[email protected]> > > Reviewed-by: Jason Gunthorpe <[email protected]> > > --- > > drivers/gpu/drm/drm_gpusvm.c | 52 > > ++++++------------------------------------ > > 1 file changed, 7 insertions(+), 45 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c > > index 958cb605aedd..6b7a6eaebcd9 100644 > > --- a/drivers/gpu/drm/drm_gpusvm.c > > +++ b/drivers/gpu/drm/drm_gpusvm.c > > @@ -788,22 +788,8 @@ 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, > > + max(timeout - jiffies, 1L)); > > if (err) > > goto err_free; > > > > @@ -1439,21 +1425,8 @@ 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, > > + max_t(long, timeout - jiffies, 1)); > > Unaligned indentation. > > So I'd write this like this to avoid weird wraps: > > ctimeout = max_t(long, timeout - jiffies, 1)); > err = hmm_range_fault_unlocked_timeout(&hmm_range, ctimeout); > > > mmput(mm); > > if (err) > > goto err_free; > > @@ -1736,24 +1709,13 @@ 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, > > + max_t(long, timeout - jiffies, 1)); > > > > Same here. > > Nits, aside LGTM. >
Will change as requested and send to [email protected] next time. Thanks, Stanislav > Matt > > > kvfree(pfns); > > mmput(mm); > > > > - return err; > > + return err == -EBUSY ? -ETIME : err; > > } > > EXPORT_SYMBOL_GPL(drm_gpusvm_range_evict); > > > > > >
