On Thu, Apr 02, 2026 at 07:11:46AM +0300, Mike Rapoport wrote: > From: "Mike Rapoport (Microsoft)" <[email protected]> > > Implementation of UFFDIO_COPY for anonymous memory might fail to copy data > from userspace buffer when the destination VMA is locked (either with > mm_lock or with per-VMA lock). > > In that case, mfill_atomic() releases the locks, retries copying the data > with locks dropped and then re-locks the destination VMA and > re-establishes PMD. > > Since this retry-reget dance is only relevant for UFFDIO_COPY and it never > happens for other UFFDIO_ operations, make it a part of > mfill_atomic_pte_copy() that actually implements UFFDIO_COPY for anonymous > memory. > > As a temporal safety measure to avoid breaking biscection > mfill_atomic_pte_copy() makes sure to never return -ENOENT so that the > loop in mfill_atomic() won't retry copiyng outside of mmap_lock. This is > removed later when shmem implementation will be updated later and the loop > in mfill_atomic() will be adjusted. > > [[email protected]: update mfill_copy_folio_retry()] > Link: https://lkml.kernel.org/r/[email protected] > Link: https://lkml.kernel.org/r/[email protected] > Signed-off-by: Mike Rapoport (Microsoft) <[email protected]> > Signed-off-by: Andrew Morton <[email protected]> > ---
Looks good to me, Reviewed-by: Harry Yoo (Oracle) <[email protected]> > mm/userfaultfd.c | 75 ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 51 insertions(+), 24 deletions(-) > > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c > index c6a38db45343..82e1a3255e1e 100644 > --- a/mm/userfaultfd.c > +++ b/mm/userfaultfd.c > @@ -405,35 +405,63 @@ static int mfill_copy_folio_locked(struct folio *folio, > unsigned long src_addr) > static int mfill_atomic_pte_copy(struct mfill_state *state) > { > - struct vm_area_struct *dst_vma = state->vma; > unsigned long dst_addr = state->dst_addr; > unsigned long src_addr = state->src_addr; > uffd_flags_t flags = state->flags; > - pmd_t *dst_pmd = state->pmd; > struct folio *folio; > int ret; > > - if (!state->folio) { > - ret = -ENOMEM; > - folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, dst_vma, > - dst_addr); > - if (!folio) > - goto out; > + folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, state->vma, dst_addr); > + if (!folio) > + return -ENOMEM; > > - ret = mfill_copy_folio_locked(folio, src_addr); > + ret = -ENOMEM; > + if (mem_cgroup_charge(folio, state->vma->vm_mm, GFP_KERNEL)) > + goto out_release; > > - /* fallback to copy_from_user outside mmap_lock */ > - if (unlikely(ret)) { > - ret = -ENOENT; > - state->folio = folio; > - /* don't free the page */ > - goto out; > - } > - } else { > - folio = state->folio; > - state->folio = NULL; > + ret = mfill_copy_folio_locked(folio, src_addr); > + if (unlikely(ret)) { > + /* > + * Fallback to copy_from_user outside mmap_lock. > + * If retry is successful, mfill_copy_folio_locked() returns > + * with locks retaken by mfill_get_vma(). nit: mfill_copy_folio_locked() -> mfill_copy_folio_retry(); > + * If there was an error, we must mfill_put_vma() anyway and it > + * will take care of unlocking if needed. > + */ > + ret = mfill_copy_folio_retry(state, folio); > + if (ret) > + goto out_release; > } > > /* -- Cheers, Harry / Hyeonggon
