Re: [PATCH v8 4/8] mshv: Use hmm_range_fault_unlocked_timeout() for region faults

[Stanislav Kinsburskii]

On Fri, Jul 10, 2026 at 03:12:16PM -0700, Andrew Morton wrote:
> On Fri, 10 Jul 2026 14:26:50 -0700 Stanislav Kinsburskii 
> <skinsburs...@gmail.com> wrote:
> 
> > MSHV currently faults movable memory regions by taking mmap_read_lock()
> > around hmm_range_fault(). That prevents the fault path from handling VMAs
> > whose fault handlers need to drop mmap_lock, such as userfaultfd-backed
> > mappings.
> > 
> > Use hmm_range_fault_unlocked_timeout() instead. Passing a timeout of 0
> > preserves MSHV's existing unbounded retry behavior while letting the HMM
> > helper own mmap_lock acquisition and refresh range->notifier_seq internally
> > before walking the range. After the fault succeeds, MSHV still takes
> > mreg_mutex and checks mmu_interval_read_retry() before installing the pages
> > into the region, so the existing invalidation synchronization is preserved.
> > 
> > Fold the small fault-and-lock helper into mshv_region_range_fault(), since
> > the remaining retry path is just the standard "fault, take the driver lock,
> > check the interval notifier sequence" pattern.
> > 
> > ...
> >
> > @@ -452,13 +412,19 @@ static int mshv_region_range_fault(struct 
> > mshv_mem_region *region,
> >     range.start = region->start_uaddr + page_offset * HV_HYP_PAGE_SIZE;
> >     range.end = range.start + page_count * HV_HYP_PAGE_SIZE;
> >  
> > -   do {
> > -           ret = mshv_region_hmm_fault_and_lock(region, &range);
> > -   } while (ret == -EBUSY);
> > -
> > +again:
> > +   ret = hmm_range_fault_unlocked_timeout(&range, 0);
> >     if (ret)
> >             goto out;
> >  
> > +   mutex_lock(&region->mreg_mutex);
> > +
> > +   if (mmu_interval_read_retry(range.notifier, range.notifier_seq)) {
> > +           mutex_unlock(&region->mreg_mutex);
> > +           cond_resched();
> > +           goto again;
> > +   }
> > +
> 
> If the calling process has realtime scheduling policy and either a)
> we're uniprocessor or b) this process and the holder of
> interval_sub->invalidate_seq are both pinned to the same CPU then
> cond_resched() won't do anything, and this might be an infinite loop?

Yes, looks like it might.
What can be done to prevent this?

Thanks,
Stanislav