Re: [PATCH 3/5] mm: Rename __thp_get_unmapped_area to mm_get_unmapped_area_aligned

Jason Gunthorpe
2025-06-16T12:14:33Z

On Sat, Jun 14, 2025 at 01:23:30AM -0400, Liam R. Howlett wrote:

> vm_unmapped_area_info has an align_mask, and that's only used for
> hugepages. It is wrong to have a generic function that does not use the
> generic struct element that exists for this reason. Is there a reason
> that align_mask doesn't work, or why it's not used?

I had the same question and looked into it for a bit. It does seem
desirable, but also not entirely straightforward. I think the arch
code for arch_get_unmapped_area() needs some redesign to produce
the vm_unmapped_area_info() that the core code can update.

Unfortunately there are numerous weird things in the arches :\

Like x86 shouldn't be setting alignment for huge tlbfs files, that
should be done in the core code by huge tlbfs caling the new
mm_get_unmapped_area_aligned() on its own..

So I think we should leave this hacky implementation for now and start
building out the generic side to call it in the right places, then we
can consider how to implement a better integration with the arch code.

Also, probably 'aligned' is not the right name. This new function
should be called by VMA owners that know they have pgoff aligned high
order folios/pfns inside their mapping. The 'align' argument is the
max order of their pgoff aligned folio/pfns.

The purpose of the function is to adjust the resulting area to
optimize for the high order folios that are present while following
the uAPI rules for mmap.

Maybe call it something like _order and document it like the above?

> I also am not okay to export it for no reason.

The next patches are the reason.

> Also, is it okay to export something as gpl or does the copyright holder
> need to do that (I have no idea about this stuff, or maybe you work for
> the copyright holder)?

Yes, you are always safe to use the GPL export.

> The hint (addr) is also never checked for alignment in this function and
> we are appending _aligned() to the name. With this change we can now
> get an unaligned _aligned() address. This (probably) can happen with
> MAP_FIXED today, but I don't think we imply it's going to be aligned
> elsewhere.

Should be documented at least..

Jason