On 2020-03-31 at 02:39 Mike Kravetz wrote: >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@xxxxxxxxx> >>>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> >>>> Cc: Mike Kravetz <mike.kravetz@xxxxxxxxxx> >>>> Cc: John Hubbard <jhubbard@xxxxxxxxxx> >>>> --- >>>> 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@xxxxxxxxx/ > >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. > Agree, it would casue regression today if those user space application still work in that way. After read through the bug report page, it is indeed for some applications want to use not aligned size for mmap(), but don't care what will happen if that size been used in subsequent calls. My understanding may wrong, but it seems that once some application start to use some behavior of kernel, although that usage in user space is not logical, they will be protected from change in kernel side. >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
