Re: [PATCH v4 06/10] powerpc/mm: Use generic_get_unmapped_area() and call it from arch_get_unmapped_area()
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2021-12-13 13:11:59
Also in:
linux-mm, lkml
Christophe Leroy [off-list ref] writes:
quoted hunk ↗ jump to hunk
Use the generic version of arch_get_unmapped_area() which is now available at all time instead of its copy radix__arch_get_unmapped_area() Instead of setting mm->get_unmapped_area() to either arch_get_unmapped_area() or generic_get_unmapped_area(), always set it to arch_get_unmapped_area() and call generic_get_unmapped_area() from there when radix is enabled. Do the same with radix__arch_get_unmapped_area_topdown() Signed-off-by: Christophe Leroy <redacted> --- arch/powerpc/mm/mmap.c | 127 ++--------------------------------------- 1 file changed, 6 insertions(+), 121 deletions(-)diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c index 9b0d6e395bc0..46781d0103d1 100644 --- a/arch/powerpc/mm/mmap.c +++ b/arch/powerpc/mm/mmap.c@@ -81,115 +81,15 @@ static inline unsigned long mmap_base(unsigned long rnd, } #ifdef HAVE_ARCH_UNMAPPED_AREA -#ifdef CONFIG_PPC_RADIX_MMU -/* - * Same function as generic code used only for radix, because we don't need to overload - * the generic one. But we will have to duplicate, because hash select - * HAVE_ARCH_UNMAPPED_AREA - */ -static unsigned long -radix__arch_get_unmapped_area(struct file *filp, unsigned long addr, - unsigned long len, unsigned long pgoff, - unsigned long flags) -{ - struct mm_struct *mm = current->mm; - struct vm_area_struct *vma; - int fixed = (flags & MAP_FIXED); - unsigned long high_limit; - struct vm_unmapped_area_info info; - - high_limit = DEFAULT_MAP_WINDOW; - if (addr >= high_limit || (fixed && (addr + len > high_limit))) - high_limit = TASK_SIZE; - - if (len > high_limit) - return -ENOMEM;
There are some differences in the above vs the generic code, the generic
arch_get_unmapped_area_topdown() in mm/mmap.c does:
const unsigned long mmap_end = arch_get_mmap_end(addr);
if (len > mmap_end - mmap_min_addr)
return -ENOMEM;
if (flags & MAP_FIXED)
return addr;
Our current code adjusts high_limit for fixed mappings that span above
the default map window. We added that logic in:
35602f82d0c7 ("powerpc/64s/hash: Allow MAP_FIXED allocations to cross 128TB boundary")
That means a fixed mapping that crosses the 128T boundary will be
allowed by our code.
On the other hand the generic code will allow a fixed mapping to cross
the 128T boundary, but only if the size of the mapping is < ~128T.
(The actual size limit is (128T - mmap_min_addr), which is usually 4K or
64K, but is adjustable.)
It's unlikely that any apps are doing fixed mappings larger than 128T
that cross the 128T boundary, but I think we need to allow it. 128T
seems like a lot, but is not compared to the entire 4PB address space.
So I think we need to fix that in the generic code.
The easiest option is probably to pass flags to arch_get_mmap_end(), and
then the arches can decide whether to adjust the return value based on
flags.
Then there's also the extra check we have here:
- if (fixed) {
- if (addr > high_limit - len)
- return -ENOMEM;
- return addr;
- }I think we can drop that when converting to the generic version, the only case in which it matters is when high_limit == TASK_SIZE, and get_unmapped_area() already does that check after calling us: if (addr > TASK_SIZE - len) return -ENOMEM; cheers