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

["Lorenzo Stoakes (Oracle)" <ljs@kernel.org>]

On Fri, Apr 03, 2026 at 11:11:25PM +0530, Sayali Patil wrote:
>
>
> 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!

Cheers, Lorenzo