arch/loongarch/include/asm/hugetlb.h | 14 -------------- arch/mips/include/asm/hugetlb.h | 14 -------------- fs/hugetlbfs/inode.c | 8 ++------ include/asm-generic/hugetlb.h | 8 -------- include/linux/hugetlb.h | 6 ------ 5 files changed, 2 insertions(+), 48 deletions(-)
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.
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Huacai Chen <chenhuacai@kernel.org>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Muchun Song <muchun.song@linux.dev>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: loongarch@lists.linux.dev
Cc: linux-mips@vger.kernel.org
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Acked-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
arch/loongarch/include/asm/hugetlb.h | 14 --------------
arch/mips/include/asm/hugetlb.h | 14 --------------
fs/hugetlbfs/inode.c | 8 ++------
include/asm-generic/hugetlb.h | 8 --------
include/linux/hugetlb.h | 6 ------
5 files changed, 2 insertions(+), 48 deletions(-)
diff --git a/arch/loongarch/include/asm/hugetlb.h b/arch/loongarch/include/asm/hugetlb.h
index 4dc4b3e04225..ab68b594f889 100644
--- a/arch/loongarch/include/asm/hugetlb.h
+++ b/arch/loongarch/include/asm/hugetlb.h
@@ -10,20 +10,6 @@
uint64_t pmd_to_entrylo(unsigned long pmd_val);
-#define __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE
-static inline int prepare_hugepage_range(struct file *file,
- unsigned long addr,
- unsigned long len)
-{
- unsigned long task_size = STACK_TOP;
-
- if (len > task_size)
- return -ENOMEM;
- if (task_size - len < addr)
- return -EINVAL;
- return 0;
-}
-
#define __HAVE_ARCH_HUGE_PTE_CLEAR
static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, unsigned long sz)
diff --git a/arch/mips/include/asm/hugetlb.h b/arch/mips/include/asm/hugetlb.h
index fbc71ddcf0f6..8c460ce01ffe 100644
--- a/arch/mips/include/asm/hugetlb.h
+++ b/arch/mips/include/asm/hugetlb.h
@@ -11,20 +11,6 @@
#include <asm/page.h>
-#define __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE
-static inline int prepare_hugepage_range(struct file *file,
- unsigned long addr,
- unsigned long len)
-{
- unsigned long task_size = STACK_TOP;
-
- if (len > task_size)
- return -ENOMEM;
- if (task_size - len < addr)
- return -EINVAL;
- return 0;
-}
-
#define __HAVE_ARCH_HUGE_PTEP_GET_AND_CLEAR
static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
unsigned long addr, pte_t *ptep,
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 00b2d1a032fd..81a6acddd690 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -179,12 +179,8 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
if (len & ~huge_page_mask(h))
return -EINVAL;
- if (flags & MAP_FIXED) {
- if (addr & ~huge_page_mask(h))
- return -EINVAL;
- if (prepare_hugepage_range(file, addr, len))
- return -EINVAL;
- }
+ if ((flags & MAP_FIXED) && (addr & ~huge_page_mask(h)))
+ return -EINVAL;
if (addr)
addr0 = ALIGN(addr, huge_page_size(h));
diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h
index 3e0a8fe9b108..4bce4f07f44f 100644
--- a/include/asm-generic/hugetlb.h
+++ b/include/asm-generic/hugetlb.h
@@ -114,14 +114,6 @@ static inline int huge_pte_none_mostly(pte_t pte)
}
#endif
-#ifndef __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE
-static inline int prepare_hugepage_range(struct file *file,
- unsigned long addr, unsigned long len)
-{
- return 0;
-}
-#endif
-
#ifndef __HAVE_ARCH_HUGE_PTEP_SET_WRPROTECT
static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
unsigned long addr, pte_t *ptep)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index c6c87eae4a8d..474de8e2a8f2 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -359,12 +359,6 @@ static inline void hugetlb_show_meminfo_node(int nid)
{
}
-static inline int prepare_hugepage_range(struct file *file,
- unsigned long addr, unsigned long len)
-{
- return -EINVAL;
-}
-
static inline void hugetlb_vma_lock_read(struct vm_area_struct *vma)
{
}
--
2.49.0
* 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. 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? 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. Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> > > Cc: Liam R. Howlett <Liam.Howlett@oracle.com> > Cc: Huacai Chen <chenhuacai@kernel.org> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Muchun Song <muchun.song@linux.dev> > Cc: Oscar Salvador <osalvador@suse.de> > Cc: loongarch@lists.linux.dev > Cc: linux-mips@vger.kernel.org > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > Reviewed-by: Oscar Salvador <osalvador@suse.de> > Acked-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > arch/loongarch/include/asm/hugetlb.h | 14 -------------- > arch/mips/include/asm/hugetlb.h | 14 -------------- > fs/hugetlbfs/inode.c | 8 ++------ > include/asm-generic/hugetlb.h | 8 -------- > include/linux/hugetlb.h | 6 ------ > 5 files changed, 2 insertions(+), 48 deletions(-) > > diff --git a/arch/loongarch/include/asm/hugetlb.h b/arch/loongarch/include/asm/hugetlb.h > index 4dc4b3e04225..ab68b594f889 100644 > --- a/arch/loongarch/include/asm/hugetlb.h > +++ b/arch/loongarch/include/asm/hugetlb.h > @@ -10,20 +10,6 @@ > > uint64_t pmd_to_entrylo(unsigned long pmd_val); > > -#define __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE > -static inline int prepare_hugepage_range(struct file *file, > - unsigned long addr, > - unsigned long len) > -{ > - unsigned long task_size = STACK_TOP; > - > - if (len > task_size) > - return -ENOMEM; > - if (task_size - len < addr) > - return -EINVAL; > - return 0; > -} > - > #define __HAVE_ARCH_HUGE_PTE_CLEAR > static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr, > pte_t *ptep, unsigned long sz) > diff --git a/arch/mips/include/asm/hugetlb.h b/arch/mips/include/asm/hugetlb.h > index fbc71ddcf0f6..8c460ce01ffe 100644 > --- a/arch/mips/include/asm/hugetlb.h > +++ b/arch/mips/include/asm/hugetlb.h > @@ -11,20 +11,6 @@ > > #include <asm/page.h> > > -#define __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE > -static inline int prepare_hugepage_range(struct file *file, > - unsigned long addr, > - unsigned long len) > -{ > - unsigned long task_size = STACK_TOP; > - > - if (len > task_size) > - return -ENOMEM; > - if (task_size - len < addr) > - return -EINVAL; > - return 0; > -} > - > #define __HAVE_ARCH_HUGE_PTEP_GET_AND_CLEAR > static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm, > unsigned long addr, pte_t *ptep, > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c > index 00b2d1a032fd..81a6acddd690 100644 > --- a/fs/hugetlbfs/inode.c > +++ b/fs/hugetlbfs/inode.c > @@ -179,12 +179,8 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr, > > if (len & ~huge_page_mask(h)) > return -EINVAL; > - if (flags & MAP_FIXED) { > - if (addr & ~huge_page_mask(h)) > - return -EINVAL; > - if (prepare_hugepage_range(file, addr, len)) > - return -EINVAL; > - } > + if ((flags & MAP_FIXED) && (addr & ~huge_page_mask(h))) > + return -EINVAL; > if (addr) > addr0 = ALIGN(addr, huge_page_size(h)); > > diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h > index 3e0a8fe9b108..4bce4f07f44f 100644 > --- a/include/asm-generic/hugetlb.h > +++ b/include/asm-generic/hugetlb.h > @@ -114,14 +114,6 @@ static inline int huge_pte_none_mostly(pte_t pte) > } > #endif > > -#ifndef __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE > -static inline int prepare_hugepage_range(struct file *file, > - unsigned long addr, unsigned long len) > -{ > - return 0; > -} > -#endif > - > #ifndef __HAVE_ARCH_HUGE_PTEP_SET_WRPROTECT > static inline void huge_ptep_set_wrprotect(struct mm_struct *mm, > unsigned long addr, pte_t *ptep) > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h > index c6c87eae4a8d..474de8e2a8f2 100644 > --- a/include/linux/hugetlb.h > +++ b/include/linux/hugetlb.h > @@ -359,12 +359,6 @@ static inline void hugetlb_show_meminfo_node(int nid) > { > } > > -static inline int prepare_hugepage_range(struct file *file, > - unsigned long addr, unsigned long len) > -{ > - return -EINVAL; > -} > - > static inline void hugetlb_vma_lock_read(struct vm_area_struct *vma) > { > } > -- > 2.49.0 > >
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
* 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
On 27/06/25 9:37 PM, Peter Xu wrote: > 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. Agreed, this API is now redundant. > > Cc: Liam R. Howlett <Liam.Howlett@oracle.com> > Cc: Huacai Chen <chenhuacai@kernel.org> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Muchun Song <muchun.song@linux.dev> > Cc: Oscar Salvador <osalvador@suse.de> > Cc: loongarch@lists.linux.dev > Cc: linux-mips@vger.kernel.org > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > Reviewed-by: Oscar Salvador <osalvador@suse.de> > Acked-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > arch/loongarch/include/asm/hugetlb.h | 14 -------------- > arch/mips/include/asm/hugetlb.h | 14 -------------- > fs/hugetlbfs/inode.c | 8 ++------ > include/asm-generic/hugetlb.h | 8 -------- > include/linux/hugetlb.h | 6 ------ > 5 files changed, 2 insertions(+), 48 deletions(-) > > diff --git a/arch/loongarch/include/asm/hugetlb.h b/arch/loongarch/include/asm/hugetlb.h > index 4dc4b3e04225..ab68b594f889 100644 > --- a/arch/loongarch/include/asm/hugetlb.h > +++ b/arch/loongarch/include/asm/hugetlb.h > @@ -10,20 +10,6 @@ > > uint64_t pmd_to_entrylo(unsigned long pmd_val); > > -#define __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE > -static inline int prepare_hugepage_range(struct file *file, > - unsigned long addr, > - unsigned long len) > -{ > - unsigned long task_size = STACK_TOP; > - > - if (len > task_size) > - return -ENOMEM; > - if (task_size - len < addr) > - return -EINVAL; > - return 0; > -} > - > #define __HAVE_ARCH_HUGE_PTE_CLEAR > static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr, > pte_t *ptep, unsigned long sz) > diff --git a/arch/mips/include/asm/hugetlb.h b/arch/mips/include/asm/hugetlb.h > index fbc71ddcf0f6..8c460ce01ffe 100644 > --- a/arch/mips/include/asm/hugetlb.h > +++ b/arch/mips/include/asm/hugetlb.h > @@ -11,20 +11,6 @@ > > #include <asm/page.h> > > -#define __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE > -static inline int prepare_hugepage_range(struct file *file, > - unsigned long addr, > - unsigned long len) > -{ > - unsigned long task_size = STACK_TOP; > - > - if (len > task_size) > - return -ENOMEM; > - if (task_size - len < addr) > - return -EINVAL; > - return 0; > -} > - > #define __HAVE_ARCH_HUGE_PTEP_GET_AND_CLEAR > static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm, > unsigned long addr, pte_t *ptep, > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c > index 00b2d1a032fd..81a6acddd690 100644 > --- a/fs/hugetlbfs/inode.c > +++ b/fs/hugetlbfs/inode.c > @@ -179,12 +179,8 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr, > > if (len & ~huge_page_mask(h)) > return -EINVAL; > - if (flags & MAP_FIXED) { > - if (addr & ~huge_page_mask(h)) > - return -EINVAL; > - if (prepare_hugepage_range(file, addr, len)) > - return -EINVAL; > - } > + if ((flags & MAP_FIXED) && (addr & ~huge_page_mask(h))) > + return -EINVAL; > if (addr) > addr0 = ALIGN(addr, huge_page_size(h)); > > diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h > index 3e0a8fe9b108..4bce4f07f44f 100644 > --- a/include/asm-generic/hugetlb.h > +++ b/include/asm-generic/hugetlb.h > @@ -114,14 +114,6 @@ static inline int huge_pte_none_mostly(pte_t pte) > } > #endif > > -#ifndef __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE > -static inline int prepare_hugepage_range(struct file *file, > - unsigned long addr, unsigned long len) > -{ > - return 0; > -} > -#endif > - > #ifndef __HAVE_ARCH_HUGE_PTEP_SET_WRPROTECT > static inline void huge_ptep_set_wrprotect(struct mm_struct *mm, > unsigned long addr, pte_t *ptep) > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h > index c6c87eae4a8d..474de8e2a8f2 100644 > --- a/include/linux/hugetlb.h > +++ b/include/linux/hugetlb.h > @@ -359,12 +359,6 @@ static inline void hugetlb_show_meminfo_node(int nid) > { > } > > -static inline int prepare_hugepage_range(struct file *file, > - unsigned long addr, unsigned long len) > -{ > - return -EINVAL; > -} > - > static inline void hugetlb_vma_lock_read(struct vm_area_struct *vma) > { > } A small nit - there is a now stale in code comment still referring to prepare_hugepage_range() in hugetlbfs_file_mmap(). Otherwise LGTM. Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
© 2016 - 2025 Red Hat, Inc.