Re: [PATCH] mm/hugetlb: Remove prepare_hugepage_range()

[Peter Xu <peterx@redhat.com>]

On Mon, Jun 30, 2025 at 10:26:13AM -0400, Liam R. Howlett wrote:
> * Peter Xu <peterx@redhat.com> [250627 12:07]:
> > Only mips and loongarch implemented this API, however what it does was
> > checking against stack overflow for either len or addr.  That's already
> > done in arch's arch_get_unmapped_area*() functions, even though it may not
> > be 100% identical checks.
> > 
> > For example, for both of the architectures, there will be a trivial
> > difference on how stack top was defined.  The old code uses STACK_TOP which
> > may be slightly smaller than TASK_SIZE on either of them, but the hope is
> > that shouldn't be a problem.
> > 
> > It means the whole API is pretty much obsolete at least now, remove it
> > completely.
> 
> Thanks for rewording this change, apologies for the late response on
> your change log.  I think it's fine.

That's totally OK, thanks for keeping an eye.

> 
> I think the only meaningful difference is that the failure would have
> aborted entirely if stack_top - len < addr, but with this change it will
> attempt to search in the opposite direction.  Unless I missed something?

IIUC the prepare_hugepage_range() API shouldn't affect the direction of VA
allocation yet, but rather a (likely proven unnecessary by this patch) way
to fail fast for hugetlbfs for no good reason.

It is currently only invoked with MAP_FIXED, and if it returns 0 it'll
further move on to the core VA allocator.  Then the allocation can happen
either TOP->DOWN or DOWN->TOP.

So "stack_top - len < addr" check is meaningful no matter if MMF_TOPDOWN or
not, because we want to make sure it won't overflow in any direction.  It's
just that it's already covered at least by the two archs providing this
hugetlbfs specific hook.

> 
> I suspect that this wasn't meant to happen in the first place anyways,
> and I bet there are no tests for it and no real-world harm in changing
> an error scenario into a potential successful mapping.

Yes, checking stack for hugetlbfs so far just looks redundant.  Maybe keep
digging the history we may found why we had it before, but so far it looks
not useful at all.

> 
> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

Thanks,

-- 
Peter Xu