On Fri, Jul 10, 2026 at 01:27:49PM -0300, Jason Gunthorpe wrote:
> On Tue, Jul 07, 2026 at 12:47:01PM -0700, Stanislav Kinsburskii wrote:
>
> > +int hmm_range_fault_unlocked_timeout(struct hmm_range *range,
> > + unsigned long timeout)
> > +{
> > + struct mm_struct *mm = range->notifier->mm;
> > + unsigned long deadline = 0;
> > + int locked, ret;
> > +
> > + if (timeout)
> > + deadline = jiffies + timeout;
> > +
> > + do {
> > + if (fatal_signal_pending(current))
> > + return -EINTR;
> > +
> > + if (timeout && time_after(jiffies, deadline))
> > + return -EBUSY;
>
> I really dislike there is a timeout here, HMM is supposed to be more
> deterministic. GUP doesn't have a timeout, what is this about?
>
The timeout was added because this version makes the unlocked helper own
the internal retry loop, including -EBUSY retries.
Several existing HMM users already bound those retries with
HMM_RANGE_DEFAULT_TIMEOUT, so the timeout argument was meant to avoid
silently turning those call sites into unbounded waits.
That said, I agree this mixes driver retry policy into HMM. A cleaner
split may be to make the new helper timeout-free and have it retry
internally only when mmap_lock was dropped by the fault path, since that
is the part HMM must hide for userfaultfd-style faults.
If the walk fails with the normal HMM invalidation -EBUSY, the helper
would return -EBUSY to the caller, preserving the existing driver
timeout loops and keeping HMM deterministic, closer to GUP.
Is it better from your POV?
THanks,
Stanislav
> Jason
