On 4/27/20 4:10 AM, Shijie Hu wrote:
> In 32-bit programs, the address space is limited. When the normal mmap
> consumes the space above TASK_UNMAPPED_BASE on legacy mode, it can still
> successfully obtain unmapped area below TASK_UNMAPPED_BASE, but mmap or
> shmat for huge pages will fail. This seems "not fair".
>
> When the request for huge pages fails, fall back to reuse mmap_min_addr
> ~ TASK_UNMAPPED_BASE for hugetlbfs.
Just curious. Have you actually seeing a problem with this code, or is
the reason for the proposed change just the result of code inspection? I ask
because many architectures have their own version of hugetlb_get_unmapped_area.
So, if you are seeing this issue it would be interesting to know what
architecture you are running.
The routine hugetlb_get_unmapped_area has not changed much since this first
git version. I suspect this is because it is mostly unused.
I noticed that hugetlb_get_unmapped_area is one of only a few places in arch
independent code calling vm_unmapped_area(). The other callers are arch
independent fall back routines for arch_get_unmapped_area* routines. If we
move forward with changes to this routine, would it make more sense to use
the arch_get_unmapped_area* routines instead of calling vm_unmapped_area
directly? This would take advantage of any arch specific if it exists.
--
Mike Kravetz
>
> Signed-off-by: Shijie Hu <hushijie3@xxxxxxxxxx>
> ---
> fs/hugetlbfs/inode.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index aff8642f0c2e..0f5997394aaa 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -224,7 +224,21 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> info.high_limit = TASK_SIZE;
> info.align_mask = PAGE_MASK & ~huge_page_mask(h);
> info.align_offset = 0;
> - return vm_unmapped_area(&info);
> + addr = vm_unmapped_area(&info);
> +
> + /*
> + * A failed request for huge pages very likely causes application
> + * failure, so fall back to the top-down function here.
> + */
> + if (unlikely(offset_in_page(addr))) {
> + VM_BUG_ON(addr != -ENOMEM);
> + info.flags = VM_UNMAPPED_AREA_TOPDOWN;
> + info.low_limit = max(PAGE_SIZE, mmap_min_addr);
> + info.high_limit = TASK_UNMAPPED_BASE;
> + addr = vm_unmapped_area(&info);
> + }
> +
> + return addr;
> }
> #endif
>
>
