Re: [RFC PATCH] mm/hmm: Add userfaultfd support to fault handling

[Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>]

On Wed, Apr 22, 2026 at 09:27:44AM +0200, David Hildenbrand (Arm) wrote:
> On 4/1/26 00:24, Stanislav Kinsburskii wrote:
> > Add support for userfaultfd-enabled VMAs to the HMM framework.
> > 
> > Extract fault handling logic into hmm_handle_mm_fault() to handle both
> > regular and userfaultfd-backed mappings. The implementation follows
> > fixup_user_fault() for handling VM_FAULT_RETRY and VM_FAULT_COMPLETED, with
> > a key difference: instead of retrying or moving forward respectively,
> > return -EBUSY after reacquiring mmap_read_lock. Since the lock was
> > released, the VMA could have changed, so defer retry logic to the caller.
> > 
> > This approach is inefficient for userfaultfd-backed VMAs, as HMM can only
> > populate one page at a time, but keeps the framework simple by avoiding
> > complex retry logic within HMM itself.
> > 
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > ---
> >  mm/hmm.c |   40 ++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 36 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/hmm.c b/mm/hmm.c
> > index f6c4ddff4bd6..d04d68e21473 100644
> > --- a/mm/hmm.c
> > +++ b/mm/hmm.c
> > @@ -59,6 +59,35 @@ static int hmm_pfns_fill(unsigned long addr, unsigned long end,
> >  	return 0;
> >  }
> >  
> > +static int hmm_handle_mm_fault(struct vm_area_struct *vma,
> > +			       unsigned long addr,
> > +			       unsigned int fault_flags)
> > +{
> > +	int ret;
> > +
> > +	if (userfaultfd_missing(vma)) {
> > +		struct mm_struct *mm = vma->vm_mm;
> > +
> > +		fault_flags |= FAULT_FLAG_ALLOW_RETRY |
> > +			       FAULT_FLAG_USER;
> > +
> > +		ret = handle_mm_fault(vma, addr, fault_flags, NULL);
> > +
> > +		if (ret & (VM_FAULT_COMPLETED | VM_FAULT_RETRY)) {
> > +			mmap_read_lock(mm);
> > +			return -EBUSY;
> > +		}
> > +
> > +		if (ret & VM_FAULT_ERROR)
> > +			return vm_fault_to_errno(ret, 0);
> > +	} else {
> > +		ret = handle_mm_fault(vma, addr, fault_flags, NULL);
> > +		if (ret & VM_FAULT_ERROR)
> > +			return vm_fault_to_errno(ret, 0);
> > +	}
> 
> I'm surprised that there is userfaultfd_missing() logic required here at
> all.
> 
> What prevents us from always calling handle_mm_fault() in a way +
> handling return values, such that we will just do the right thing
> independent of userfaultfd_missing()?
> 

Essentially, the main issue with the current HMM framework is that
handle_mm_fault() with FAULT_FLAG_ALLOW_RETRY (needed for userfaultfd)
can drop the mm lock. The framework does not expect or support that yet.
That is why I had to add the branching.

The worst part is that handle_mm_fault() can unlock via
mm_read_unlock(), while svm_range_restore_work() takes the mm write
lock and this RFC patch doesn't address this problem.

The main question where I’d like feedback is: should userfaultfd be
supported in the HMM framework at all, given the complexity around
unlocking and retry handling (similar to other gup helpers) or should
this complexity be offloaded to the caller side?

If it should be suported, then I see two ways to implement this:

1. Introduce a new HMM_NEED_USER_FAULT flag to tell the framework that
   the caller needs userfaultfd support, and extend the framework
   accordingly. This is not intrusive to existing code. But if we want lazy
   migration for GPU state in the future (and we likely do), GPU drivers
   will have to be updated to set this flag later. The only real user of
   this feature right now is the MSHV driver and I'd extend it to set
   this flag right away and leave GPU HMM users for later.

2. Add retry logic to hmm_handle_mm_fault() to handle VM_FAULT_RETRY and
   VM_FAULT_COMPLETED, as required for userfaultfd support. This would be
   transparent to users. But it would require significant changes: mm
   relocking, VMA lookup, and other parts. Also, before doing that, the
   kfd_svm driver would need to be changed to downgrade the mm lock to
   read.

Thanks,
Stanislav

> -- 
> Cheers,
> 
> David