Re: [PATCH v3 08/13] selftests/mm: ensure destination is hugetlb-backed in hugepage-mremap

[Sayali Patil]

On 02/04/26 14:35, Lorenzo Stoakes (Oracle) wrote:
> On Thu, Apr 02, 2026 at 09:33:29AM +0200, David Hildenbrand (Arm) wrote:
> > On 4/1/26 22:39, Sayali Patil wrote:
> > > On 01/04/26 20:10, Lorenzo Stoakes (Oracle) wrote:
> > > > On Wed, Apr 01, 2026 at 04:21:55PM +0200, David Hildenbrand (Arm) wrote:
> > > >
> > > > OK so digging in:
> > > >
> > > > mremap -> ... -> vrm_set_new_addr() -> get_unmapped_area() -> ... (in
> > > > ppc arch
> > > > code) -> slice_get_unmapped_area():
> > > >
> > > > unsigned long slice_get_unmapped_area(unsigned long addr, unsigned
> > > > long len,
> > > >                        unsigned long flags, unsigned int psize,
> > > >                        int topdown)
> > > > {
> > > >      ...
> > > >      /* bunch of checks */
> > > >
> > > >      /* If we have MAP_FIXED and failed the above steps, then error out */
> > > >      if (fixed)
> > > >          return -EBUSY;
> > > >
> > > >      ...
> > > > }
> > > >
> > > > Is presumably where we hit the issue.
> > > >
> > > > > That is weird. An mremap(MREMAP_FIXED) is really just an munmap() +
> > > > > move.
> > > >
> > > > Yeah the weird bit I guess is that we _still_ invoke
> > > > get_unmapped_area() but
> > > > with MAP_FIXED set to indicate that we want the specific address, so it's
> > > > subject to the above checks.
> > > >
> > > > > Are we sure this is not some actual problem in the hugetlb
> > > > > implementation?
> > > >
> > > > It seems the 'slices' check sees if the _target address_ has an
> > > > equivalent page
> > > > size, presumably hugetlb-mandated, and fails if they're not
> > > > equivalent, so this
> > > > change is just accounting for that.
> > > Yes, this change accounts for that by ensuring the destination is
> > > created with MAP_HUGETLB so it has the same page size as the source.
> >
> > Okay, weird, so it's the right thing to do to cover all odd arch behavior.
> >
> > > > > But then the test suddenly requires more hugetlb pages, no? I don't see
> > > > > a good reason for the MAP_POPULATE, really. It will be discarded
> > > > > either way.
> > > >
> > > > Yeah I'm not sure about the MAP_POPULATE being all that important here.
> > > As far as I understand, without MAP_POPULATE, memory accesses would
> > > trigger userfaults, and since the test is single-threaded and has no
> > > background handler for the uffd, it would deadlock. MAP_POPULATE ensures
> > > the test runs correctly by prefaulting all pages, but please let me know
> > > if I’m mistaken.
> >
> > So you are saying the test would deadlock if you are not adding
> > MAP_POPULATE? If so, please double check if that is actually the case.
> >
> > And if it's actually the case, please carefully document that in the
> > patch description, and probably as a comment above the MAP_POPULATE usage.
>
> Do keep in mind MAP_POPULATE is not _guaranteed_ to work :)
>
> For guaranteed populate you need madvise(..., MADV_POPULATE_[READ/WRITE]) or to
> directly fault in.
>
> > --
> > Cheers,
> >
> > David
>
> Cheers, Lorenzo
Thanks David and Lorenzo for the input.
I tested without MAP_POPULATE and the test works fine without it.
I will remove it in the next version.

Thanks,
Sayali