Re: [PATCH] mm: allow checking length for hugetlb mapping in mmap()

[Mike Kravetz <mike.kravetz@oracle.com>]

On 3/29/20 1:09 AM, Li Xinhai wrote:
> On 2020-03-29 at 11:53 John Hubbard wrote:
>> On 3/28/20 8:08 PM, Li Xinhai wrote:
>>> In current code, the vma related call of hugetlb mapping, except mmap,
>>> are all consider not correctly aligned length as invalid parameter,
>>> including mprotect,munmap, mlock, etc., by checking through
>>> hugetlb_vm_op_split. So, user will see failure, after successfully call
>>> mmap, although using same length parameter to other mapping syscall.
>>>
>>> It is desirable for all hugetlb mapping calls have consistent behavior,
>>> without mmap as exception(which round up length to align underlying
>>> hugepage size). In current Documentation/admin-guide/mm/hugetlbpage.rst,
>>> the description is:
>>> "
>>> Syscalls that operate on memory backed by hugetlb pages only have their
>>> lengths aligned to the native page size of the processor; they will
>>> normally fail with errno set to EINVAL or exclude hugetlb pages that
>>> extend beyond the length if not hugepage aligned. For example, munmap(2)
>>> will fail if memory is backed by a hugetlb page and the length is smaller
>>> than the hugepage size.
>>> "
>>> which express the consistent behavior.
>>
>>
>> Missing here is a description of what the patch actually does...
>>
> 
> right, more statement can be added like:
> "
> After this patch, all hugetlb mapping related syscall wil only align
> length parameter to the native page size of the processor. For mmap(),
> hugetlb_get_unmmaped_area() will set errno to EINVAL if length is not
> aligned to underlying hugepage size.
> "
> 
>>>
>>> Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Mike Kravetz <mike.kravetz@oracle.com>
>>> Cc: John Hubbard <jhubbard@nvidia.com>
>>> ---
>>> changes:
>>> 0. patch which introduce new flag for mmap()
>>>      The new flag should be avoided.
>>> https://lore.kernel.org/linux-mm/1585313944-8627-1-git-send-email-lixinhai.lxh@gmail.com/

It is not exactly clear in your commit message, but this change will cause
mmap() of hugetlb ranges to fail (-EINVAL) if length is not a multiple of
huge page size.  The mmap man page says:

  Huge page (Huge TLB) mappings
       For mappings that employ huge pages, the requirements for the arguments
       of  mmap()  and munmap() differ somewhat from the requirements for map‐
       pings that use the native system page size.

       For mmap(), offset must be a multiple of the underlying huge page size.
       The system automatically aligns length to be a multiple of the underly‐
       ing huge page size.

       For munmap(), addr and length must both be a multiple of the underlying
       huge page size.

So this change may cause application failure.  The code you are removing was
added with commit af73e4d9506d.  The commit message for that commit says:

    hugetlbfs: fix mmap failure in unaligned size request
    
    The current kernel returns -EINVAL unless a given mmap length is
    "almost" hugepage aligned.  This is because in sys_mmap_pgoff() the
    given length is passed to vm_mmap_pgoff() as it is without being aligned
    with hugepage boundary.
    
    This is a regression introduced in commit 40716e29243d ("hugetlbfs: fix
    alignment of huge page requests"), where alignment code is pushed into
    hugetlb_file_setup() and the variable len in caller side is not changed.

The change in commit af73e4d9506d was added because causing mmap to return
-EINVAL if length is not a multiple of huge page size was considered a
regression.  It would still be considered a regression today.

I understand that the behavior not consistent.  However, it is clearly
documented.  I do not believe we can change the behavior of this code.

-- 
Mike Kravetz