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. >-- >Mike Kravetz
