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

["Liam R. Howlett" <Liam.Howlett@oracle.com>]

* Peter Xu <peterx@redhat.com> [250702 13:20]:
> 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.

What I meant was, before your change the failure would happen before the
we get to the core VA allocator, so the reverse direction would never
happen.

If this is only used with MAP_FIXED, then I don't think it matters as
many tests are skipped anyways.  It might be worth mentioning that the
MAP_FIXED flag is checked in different orders based on the arch, again I
think this change resulted from forked archs cloning the code and
keeping up to date.

Thanks,
Liam