Re: [PATCH] [RFC]hugetlbfs: Get unmapped area below TASK_UNMAPPED_BASE for hugetlbfs

[Mike Kravetz]

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
>  
>