* Re: [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range()
2025-06-13 13:41 ` [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range() Peter Xu
@ 2025-06-13 14:12 ` Jason Gunthorpe
2025-06-13 14:59 ` Oscar Salvador
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2025-06-13 14:12 UTC (permalink / raw)
To: Peter Xu
Cc: linux-kernel, linux-mm, kvm, Andrew Morton, Alex Williamson,
Zi Yan, Alex Mastro, David Hildenbrand, Nico Pache, Huacai Chen,
Thomas Bogendoerfer, Muchun Song, Oscar Salvador, loongarch,
linux-mips
On Fri, Jun 13, 2025 at 09:41:08AM -0400, 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, hence not needed.
>
> It means the whole API is pretty much obsolete at least now, remove it
> completely.
>
> 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
> 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(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range()
2025-06-13 13:41 ` [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range() Peter Xu
2025-06-13 14:12 ` Jason Gunthorpe
@ 2025-06-13 14:59 ` Oscar Salvador
2025-06-13 15:13 ` Zi Yan
2025-06-14 4:11 ` Liam R. Howlett
3 siblings, 0 replies; 8+ messages in thread
From: Oscar Salvador @ 2025-06-13 14:59 UTC (permalink / raw)
To: Peter Xu
Cc: linux-kernel, linux-mm, kvm, Andrew Morton, Alex Williamson,
Zi Yan, Jason Gunthorpe, Alex Mastro, David Hildenbrand,
Nico Pache, Huacai Chen, Thomas Bogendoerfer, Muchun Song,
loongarch, linux-mips
On Fri, Jun 13, 2025 at 09:41:08AM -0400, 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, hence not needed.
>
> It means the whole API is pretty much obsolete at least now, remove it
> completely.
>
> 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
> Signed-off-by: Peter Xu <peterx@redhat.com>
I think I forgot to clean these up when I unified the unmapped_area for
hugetlb.
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Thanks Peter!
--
Oscar Salvador
SUSE Labs
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range()
2025-06-13 13:41 ` [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range() Peter Xu
2025-06-13 14:12 ` Jason Gunthorpe
2025-06-13 14:59 ` Oscar Salvador
@ 2025-06-13 15:13 ` Zi Yan
2025-06-13 16:24 ` Peter Xu
2025-06-14 4:11 ` Liam R. Howlett
3 siblings, 1 reply; 8+ messages in thread
From: Zi Yan @ 2025-06-13 15:13 UTC (permalink / raw)
To: Peter Xu
Cc: linux-kernel, linux-mm, kvm, Andrew Morton, Alex Williamson,
Jason Gunthorpe, Alex Mastro, David Hildenbrand, Nico Pache,
Huacai Chen, Thomas Bogendoerfer, Muchun Song, Oscar Salvador,
loongarch, linux-mips
On 13 Jun 2025, at 9:41, 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, hence not needed.
>
> It means the whole API is pretty much obsolete at least now, remove it
> completely.
>
> 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
> 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;
arch_get_unmapped_area_topdown() has this check.
> - if (task_size - len < addr)
> - return -EINVAL;
For this one, arch_get_unmapped_area_topdown() instead will try to
provide a different addr if the check fails.
So this patch changes the original code behavior, right?
If yes, it is worth spelling it out in the commit log.
Otherwise, Reviewed-by: Zi Yan <ziy@nvidia.com>
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range()
2025-06-13 15:13 ` Zi Yan
@ 2025-06-13 16:24 ` Peter Xu
2025-06-13 18:01 ` David Hildenbrand
0 siblings, 1 reply; 8+ messages in thread
From: Peter Xu @ 2025-06-13 16:24 UTC (permalink / raw)
To: Zi Yan
Cc: linux-kernel, linux-mm, kvm, Andrew Morton, Alex Williamson,
Jason Gunthorpe, Alex Mastro, David Hildenbrand, Nico Pache,
Huacai Chen, Thomas Bogendoerfer, Muchun Song, Oscar Salvador,
loongarch, linux-mips
On Fri, Jun 13, 2025 at 11:13:50AM -0400, Zi Yan wrote:
> On 13 Jun 2025, at 9:41, 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, hence not needed.
> >
> > It means the whole API is pretty much obsolete at least now, remove it
> > completely.
> >
> > 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
> > 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;
>
> arch_get_unmapped_area_topdown() has this check.
>
> > - if (task_size - len < addr)
> > - return -EINVAL;
>
> For this one, arch_get_unmapped_area_topdown() instead will try to
> provide a different addr if the check fails.
>
> So this patch changes the original code behavior, right?
It almost shouldn't change. Note that prepare_hugepage_range() is only
used for MAP_FIXED before this patch:
hugetlb_get_unmapped_area():
if (flags & MAP_FIXED) {
if (addr & ~huge_page_mask(h))
return -EINVAL;
if (prepare_hugepage_range(file, addr, len))
return -EINVAL;
}
Then for MAP_FIXED, on MIPS:
arch_get_unmapped_area_common():
...
if (flags & MAP_FIXED) {
/* Even MAP_FIXED mappings must reside within TASK_SIZE */
if (TASK_SIZE - len < addr)
return -EINVAL;
...
}
But if we want to be super accurate, it's indeed different, in that the old
hugetlb code was checking stack top with STACK_TOP, which is
mips_stack_top() for MIPS: it's a value that might be slightly less than
TASK_SIZE..
So strictly speaking, there's indeed a trivial difference on the oddity of
defining stack top, but my guess is nothing will be affected. I can add
some explanation into the commit message in that case.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range()
2025-06-13 16:24 ` Peter Xu
@ 2025-06-13 18:01 ` David Hildenbrand
0 siblings, 0 replies; 8+ messages in thread
From: David Hildenbrand @ 2025-06-13 18:01 UTC (permalink / raw)
To: Peter Xu, Zi Yan
Cc: linux-kernel, linux-mm, kvm, Andrew Morton, Alex Williamson,
Jason Gunthorpe, Alex Mastro, Nico Pache, Huacai Chen,
Thomas Bogendoerfer, Muchun Song, Oscar Salvador, loongarch,
linux-mips
>
> But if we want to be super accurate, it's indeed different, in that the old
> hugetlb code was checking stack top with STACK_TOP, which is
> mips_stack_top() for MIPS: it's a value that might be slightly less than
> TASK_SIZE..
>
> So strictly speaking, there's indeed a trivial difference on the oddity of
> defining stack top, but my guess is nothing will be affected. I can add
> some explanation into the commit message in that case.
Yeah, that would be good.
Acked-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range()
2025-06-13 13:41 ` [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range() Peter Xu
` (2 preceding siblings ...)
2025-06-13 15:13 ` Zi Yan
@ 2025-06-14 4:11 ` Liam R. Howlett
2025-06-17 21:07 ` Peter Xu
3 siblings, 1 reply; 8+ messages in thread
From: Liam R. Howlett @ 2025-06-14 4:11 UTC (permalink / raw)
To: Peter Xu
Cc: linux-kernel, linux-mm, kvm, Andrew Morton, Alex Williamson,
Zi Yan, Jason Gunthorpe, Alex Mastro, David Hildenbrand,
Nico Pache, Huacai Chen, Thomas Bogendoerfer, Muchun Song,
Oscar Salvador, loongarch, linux-mips
* Peter Xu <peterx@redhat.com> [691231 23:00]:
> 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, hence not needed.
I'm not as confident..
>
> It means the whole API is pretty much obsolete at least now, remove it
> completely.
>
> 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
> 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;
arch/mips/include/asm/processor.h:#define STACK_TOP mips_stack_top()
unsigned long mips_stack_top(void)
{
unsigned long top = TASK_SIZE & PAGE_MASK;
if (IS_ENABLED(CONFIG_MIPS_FP_SUPPORT)) {
/* One page for branch delay slot "emulation" */
top -= PAGE_SIZE;
}
/* Space for the VDSO, data page & GIC user page */
top -= PAGE_ALIGN(current->thread.abi->vdso->size);
top -= PAGE_SIZE;
top -= mips_gic_present() ? PAGE_SIZE : 0;
/* Space for cache colour alignment */
if (cpu_has_dc_aliases)
top -= shm_align_mask + 1;
/* Space to randomize the VDSO base */
if (current->flags & PF_RANDOMIZE)
top -= VDSO_RANDOMIZE_SIZE;
return top;
}
This seems different than TASK_SIZE.
Code is from:
commit ea7e0480a4b695d0aa6b3fa99bd658a003122113
Author: Paul Burton <paulburton@kernel.org>
Date: Tue Sep 25 15:51:26 2018 -0700
> - if (len > task_size)
> - return -ENOMEM;
> - if (task_size - len < addr)
> - return -EINVAL;
> - return 0;
> -}
> -
Unfortunately, the commit message for the addition of this code are not
helpful.
commit 50a41ff292fafe1e937102be23464b54fed8b78c
Author: David Daney <ddaney@caviumnetworks.com>
Date: Wed May 27 17:47:42 2009 -0700
... But the dates are helpful. This code used to use:
#define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE)
It's not exactly task size either.
I don't think this is an issue to remove this check because the overflow
should be caught later (or trigger the opposite search). But it's not
clear why STACK_TOP was done in the first place.. Maybe just because we
know the overflow here would be an issue later, but then we'd avoid the
opposite search - and maybe that's the point?
Either way, your comment about the same check existing doesn't seem
correct.
I haven't checked loong arch, but I'd be willing to wager this was just
cloned mips code... because this happens so much.
...
Thanks,
Liam
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/5] mm/hugetlb: Remove prepare_hugepage_range()
2025-06-14 4:11 ` Liam R. Howlett
@ 2025-06-17 21:07 ` Peter Xu
0 siblings, 0 replies; 8+ messages in thread
From: Peter Xu @ 2025-06-17 21:07 UTC (permalink / raw)
To: Liam R. Howlett, linux-kernel, linux-mm, kvm, Andrew Morton,
Alex Williamson, Zi Yan, Jason Gunthorpe, Alex Mastro,
David Hildenbrand, Nico Pache, Huacai Chen, Thomas Bogendoerfer,
Muchun Song, Oscar Salvador, loongarch, linux-mips
On Sat, Jun 14, 2025 at 12:11:22AM -0400, Liam R. Howlett wrote:
> * Peter Xu <peterx@redhat.com> [691231 23:00]:
> > 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, hence not needed.
>
> I'm not as confident..
>
> >
> > It means the whole API is pretty much obsolete at least now, remove it
> > completely.
> >
> > 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
> > 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;
>
> arch/mips/include/asm/processor.h:#define STACK_TOP mips_stack_top()
>
>
> unsigned long mips_stack_top(void)
> {
> unsigned long top = TASK_SIZE & PAGE_MASK;
>
> if (IS_ENABLED(CONFIG_MIPS_FP_SUPPORT)) {
> /* One page for branch delay slot "emulation" */
> top -= PAGE_SIZE;
> }
>
> /* Space for the VDSO, data page & GIC user page */
> top -= PAGE_ALIGN(current->thread.abi->vdso->size);
> top -= PAGE_SIZE;
> top -= mips_gic_present() ? PAGE_SIZE : 0;
>
> /* Space for cache colour alignment */
> if (cpu_has_dc_aliases)
> top -= shm_align_mask + 1;
>
> /* Space to randomize the VDSO base */
> if (current->flags & PF_RANDOMIZE)
> top -= VDSO_RANDOMIZE_SIZE;
>
> return top;
> }
>
> This seems different than TASK_SIZE.
>
> Code is from:
> commit ea7e0480a4b695d0aa6b3fa99bd658a003122113
> Author: Paul Burton <paulburton@kernel.org>
> Date: Tue Sep 25 15:51:26 2018 -0700
>
>
> > - if (len > task_size)
> > - return -ENOMEM;
> > - if (task_size - len < addr)
> > - return -EINVAL;
> > - return 0;
> > -}
> > -
>
> Unfortunately, the commit message for the addition of this code are not
> helpful.
>
> commit 50a41ff292fafe1e937102be23464b54fed8b78c
> Author: David Daney <ddaney@caviumnetworks.com>
> Date: Wed May 27 17:47:42 2009 -0700
>
> ... But the dates are helpful. This code used to use:
> #define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE)
>
> It's not exactly task size either.
>
> I don't think this is an issue to remove this check because the overflow
> should be caught later (or trigger the opposite search). But it's not
> clear why STACK_TOP was done in the first place.. Maybe just because we
> know the overflow here would be an issue later, but then we'd avoid the
> opposite search - and maybe that's the point?
>
> Either way, your comment about the same check existing doesn't seem
> correct.
I will fix up the commit message to mention both archs:
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.
>
> I haven't checked loong arch, but I'd be willing to wager this was just
> cloned mips code... because this happens so much.
They define STACK_TOP differently, but AFAIU there're some duplications in
pattern of the two archs.
Please let me know if the fixed commit message works for you above, thanks.
--
Peter Xu
^ permalink raw reply [flat|nested] 8+ messages in thread