On Thu, Oct 24, 2024 at 04:47:54PM +0100, Lorenzo Stoakes wrote:
[snip]
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index 9c0fb43064b5..a5297cfb1dfc 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -900,7 +900,8 @@ __get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
> >
> > if (get_area) {
> > addr = get_area(file, addr, len, pgoff, flags);
> > - } else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
> > + } else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)
> > + && IS_ALIGNED(len, PMD_SIZE)) {
>
> So doing this feels right but...
>
> Hm this seems like it belongs in __thp_get_unmapped_area() which does a bunch of
> checks up front returning 0 if they fail, which then results in it peforming the
> normal get unmapped area logic.
>
> That also has a bunch of (offset) alignment checks as well overflow checks
> so it would seem the natural place to also check length?
>
OK having said that, I see this function is referenced from a bunch of fs
stuff we probably don't want to potentially break by enforcing this
requirement there (at least in this fix).
So disregard that and since this looks otherwise good to me, feel free to add:
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> > /* Ensures that larger anonymous mappings are THP aligned. */
> > addr = thp_get_unmapped_area_vmflags(file, addr, len,
> > pgoff, flags, vm_flags);
> > --
> > 2.47.0
> >
Thanks!
