| Message ID | 20181017163459.20175-2-steve.capper@arm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | 52-bit userspace VAs | expand |
On Wed, Oct 17, 2018 at 05:34:56PM +0100, Steve Capper wrote: > This patch adds support for "high" userspace addresses that are > optionally supported on the system and have to be requested via a hint > mechanism ("high" addr parameter to mmap). > > Rather than duplicate the arch_get_unmapped_* stock implementations, > this patch instead introduces two architectural helper macros and > applies them to arch_get_unmapped_*: > arch_get_mmap_end(addr) - get mmap upper limit depending on addr hint > arch_get_mmap_base(addr, base) - get mmap_base depending on addr hint > > If these macros are not defined in architectural code then they default > to (TASK_SIZE) and (base) so should not introduce any behavioural > changes to architectures that do not define them. Can you explain (in the changelog) why we need to do this for arm64 when it wasn't needed for the equivalent feature on x86-64? I think the answer is that x86-64 already has its own arch_get_unmapped* functions and rather than duplicating arch_get_unmapped* for arm64, you want to continue using the generic ones with just this minor hooking. But I'd like that spelled out explicitly for the next person who comes along and wonders.
On Wed, Oct 17, 2018 at 09:48:15AM -0700, Matthew Wilcox wrote: > On Wed, Oct 17, 2018 at 05:34:56PM +0100, Steve Capper wrote: > > This patch adds support for "high" userspace addresses that are > > optionally supported on the system and have to be requested via a hint > > mechanism ("high" addr parameter to mmap). > > > > Rather than duplicate the arch_get_unmapped_* stock implementations, > > this patch instead introduces two architectural helper macros and > > applies them to arch_get_unmapped_*: > > arch_get_mmap_end(addr) - get mmap upper limit depending on addr hint > > arch_get_mmap_base(addr, base) - get mmap_base depending on addr hint > > > > If these macros are not defined in architectural code then they default > > to (TASK_SIZE) and (base) so should not introduce any behavioural > > changes to architectures that do not define them. > > Can you explain (in the changelog) why we need to do this for arm64 > when it wasn't needed for the equivalent feature on x86-64? I think the > answer is that x86-64 already has its own arch_get_unmapped* functions and > rather than duplicating arch_get_unmapped* for arm64, you want to continue > using the generic ones with just this minor hooking. But I'd like that > spelled out explicitly for the next person who comes along and wonders. > Thanks Matthew, Yes we thought it better to make an unobtrusive change to the generic code rather than copy it over to arm64 (whilst x86 already had an architecture specific version that was patched). I will make the commit log a lot clearer. Cheers,
diff --git a/mm/mmap.c b/mm/mmap.c index 5f2b2b184c60..396b8ae12783 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -2042,6 +2042,15 @@ unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info) return gap_end; } + +#ifndef arch_get_mmap_end +#define arch_get_mmap_end(addr) (TASK_SIZE) +#endif + +#ifndef arch_get_mmap_base +#define arch_get_mmap_base(addr, base) (base) +#endif + /* Get an address range which is currently unmapped. * For shmat() with addr=0. * @@ -2061,8 +2070,9 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr, struct mm_struct *mm = current->mm; struct vm_area_struct *vma, *prev; struct vm_unmapped_area_info info; + const unsigned long mmap_end = arch_get_mmap_end(addr); - if (len > TASK_SIZE - mmap_min_addr) + if (len > mmap_end - mmap_min_addr) return -ENOMEM; if (flags & MAP_FIXED) @@ -2071,7 +2081,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr, if (addr) { addr = PAGE_ALIGN(addr); vma = find_vma_prev(mm, addr, &prev); - if (TASK_SIZE - len >= addr && addr >= mmap_min_addr && + if (mmap_end - len >= addr && addr >= mmap_min_addr && (!vma || addr + len <= vm_start_gap(vma)) && (!prev || addr >= vm_end_gap(prev))) return addr; @@ -2080,7 +2090,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr, info.flags = 0; info.length = len; info.low_limit = mm->mmap_base; - info.high_limit = TASK_SIZE; + info.high_limit = mmap_end; info.align_mask = 0; return vm_unmapped_area(&info); } @@ -2100,9 +2110,10 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0, struct mm_struct *mm = current->mm; unsigned long addr = addr0; struct vm_unmapped_area_info info; + const unsigned long mmap_end = arch_get_mmap_end(addr); /* requested length too big for entire address space */ - if (len > TASK_SIZE - mmap_min_addr) + if (len > mmap_end - mmap_min_addr) return -ENOMEM; if (flags & MAP_FIXED) @@ -2112,7 +2123,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0, if (addr) { addr = PAGE_ALIGN(addr); vma = find_vma_prev(mm, addr, &prev); - if (TASK_SIZE - len >= addr && addr >= mmap_min_addr && + if (mmap_end - len >= addr && addr >= mmap_min_addr && (!vma || addr + len <= vm_start_gap(vma)) && (!prev || addr >= vm_end_gap(prev))) return addr; @@ -2121,7 +2132,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0, info.flags = VM_UNMAPPED_AREA_TOPDOWN; info.length = len; info.low_limit = max(PAGE_SIZE, mmap_min_addr); - info.high_limit = mm->mmap_base; + info.high_limit = arch_get_mmap_base(addr, mm->mmap_base); info.align_mask = 0; addr = vm_unmapped_area(&info); @@ -2135,7 +2146,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0, VM_BUG_ON(addr != -ENOMEM); info.flags = 0; info.low_limit = TASK_UNMAPPED_BASE; - info.high_limit = TASK_SIZE; + info.high_limit = mmap_end; addr = vm_unmapped_area(&info); }
This patch adds support for "high" userspace addresses that are optionally supported on the system and have to be requested via a hint mechanism ("high" addr parameter to mmap). Rather than duplicate the arch_get_unmapped_* stock implementations, this patch instead introduces two architectural helper macros and applies them to arch_get_unmapped_*: arch_get_mmap_end(addr) - get mmap upper limit depending on addr hint arch_get_mmap_base(addr, base) - get mmap_base depending on addr hint If these macros are not defined in architectural code then they default to (TASK_SIZE) and (base) so should not introduce any behavioural changes to architectures that do not define them. Signed-off-by: Steve Capper <steve.capper@arm.com> --- Changed in V2, these alterations are made to mm/mmap.c rather than copied over to arch/arm64 and modified there. --- mm/mmap.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)