On Thu, Nov 27, 2025 at 09:10:56AM -0500, Peter Xu wrote:
On Thu, Nov 27, 2025 at 01:18:10PM +0200, Mike Rapoport wrote:
On Tue, Nov 25, 2025 at 02:21:16PM -0500, Peter Xu wrote:
Hi, Mike,
On Tue, Nov 25, 2025 at 08:38:38PM +0200, Mike Rapoport wrote:
From: "Mike Rapoport (Microsoft)" rppt@kernel.org
When a VMA is registered with userfaulfd in minor mode, its ->fault() method should check if a folio exists in the page cache and if yes ->fault() should call handle_userfault(VM_UFFD_MISSING).
s/MISSING/MINOR/
Thanks, fixed.
new VM_FAULT_UFFD_MINOR there instead.
Personally I'd keep the fault path as simple as possible, because that's the more frequently used path (rather than when userfaultfd is armed). I also see it slightly a pity that even with flags introduced, it only solves the MINOR problem, not MISSING.
With David's suggestion the likely path remains unchanged.
It is not about the likely, it's about introducing flags into core path that makes the core path harder to follow, when it's not strictly required.
ret = vma->vm_ops->fault(vmf); if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_RETRY | VM_FAULT_DONE_COW | VM_FAULT_UFFD_MINOR))) { if (ret & VM_FAULT_UFFD_MINOR) return handle_userfault(vmf, VM_UFFD_MINOR); return ret; }
isn't hard to follow and it's cleaner than adding EXPORT_SYMBOL that is not strictly required.
Meanwhile, personally I'm also not sure if we should have "unlikely" here.. My gut feeling is in reality we will only have two major use cases:
(a) when userfaultfd minor isn't in the picture
(b) when userfaultfd minor registered and actively being used (e.g. in a postcopy process)
Then without likely, IIUC the hardware should optimize path selected hence both a+b performs almost equally well.
unlikely() adds a branch that hardware will predict correctly if UFFD_MINOR is actively used.
But even misspredicted branch is nothing compared to putting a task on a wait queue and waiting for userspace to react to the fault notification before handle_userfault() returns the control to the fault handler.
Just to mention, if we want, I think we have at least one more option to do the same thing, but without even introducing a new flag to ->fault() retval.
That is, when we have get_folio() around, we can essentially do two faults in sequence, one lighter then the real one, only for minor vmas, something like (I didn't think deeper, so only a rough idea shown):
__do_fault(): if (uffd_minor(vma)) { ... folio = vma->get_folio(...); if (folio) return handle_userfault(vmf, VM_UFFD_MINOR); // fallthrough, which imply a cache miss } ret = vma->vm_ops->fault(vmf);
That's something to consider for the future, especially if we'd be able to pull out MISSING handling as well from ->fault() handlers.
Thanks,
Peter Xu