On 2020-03-28 at 10:19 Li Xinhai wrote: >On 2020-03-28 at 09:31 Mike Kravetz wrote: >>On 3/27/20 12:12 PM, John Hubbard wrote: >>> On 3/27/20 5:59 AM, Li Xinhai wrote: >>>> The purpose of MAP_FIXED_HUGETLB_LEN is to check whether the parameter >>>> length is valid or not according to the target file's huge page size. >>>> When it is used, if length is not aligned to underlying huge page size, >>>> mmap() is failed with errno set to EINVAL. When it is not used, the >>>> current semantic is maintained, i.e., length is round up to underlying >>>> huge page size. >>>> >>>> In current code, the vma related call, 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. >>>> >>>> With MAP_FIXED_HUGETLB_LEN, user can choose to check if length is >>>> correctly aligned at first place when call mmap, instead of failure after >>>> mapping has been created. >>> >>> Hi Li, >>> >>> This is not worth creating a new MAP_ flag. If you look at the existing flags >>> you will see that they are both limited and carefully chosen, so as to cover >>> a reasonable chunk of functionality per flag. We don't just drop in a flag >>> for tiny corner cases like this one. >>> >>> btw, remember that user API changes require man pages updates as well. And >>> that the API has to be supported forever. And that if we use up valuable >>> flag slots on trivia then we'll run out of flags quite soon, and won't be >>> able to do broader, more important upgrades. >>> >>> Also, we need to include a user space API mailing list for things that >>> affect that. Adding them now: Linux API <linux-api@xxxxxxxxxxxxxxx> >>> The man pages mailing list will also be needed if we go there. >>> >>> Let's take a closer look at your problem and see what it takes to solve it. >>> If we need some sort of flag to mmap() or other routines, fine. But so far, >>> I can see at least two solutions that are much easier: >> >>I too question the motivation for this patch. Is it simply to eliminate some >>of the hugetlb special behavior and make it behave more like the rest of mm? >> >>> Solution idea #2: just do the length check unconditionally here (without looking >>> at a new flag), and return an error if it is not aligned. And same thing for the >>> MAP_HUGETLB case below. And delete the "len = ALIGN(len, huge_page_size(hs));" in >>> both cases. >>> >>> That would still require a man page update, and consensus that it won't Break >>> The World, but it's possible (I really don't know) that this is a more common >>> and desirable behavior. >>> >>> Let's see if anyone else weighs in about this. >> >>That certainly would be the easiest thing to do. However, I'm guessing >>the current behavior was added when hugetlb mmap support was added. > >>There is no telling how many applications might break if we change the behavior. >>I'm guessing this is the reason Li chose to only change the behavior if >>a new flag was specified. >Yes, I was considering this change would break something. > It's better to have a new patch which don't introduce new flag, and I attached statements from the kernel document. https://lore.kernel.org/linux-api/1585451295-22302-1-git-send-email-lixinhai.lxh@xxxxxxxxx/ Let's have further check, thanks! >>-- >>Mike Kravetz
