Messages in this thread | | | Date | Mon, 7 Feb 2022 15:47:04 -0800 | Subject | Re: [PATCH v2 1/3] mm: enable MADV_DONTNEED for hugetlb mappings | From | Mike Kravetz <> |
| |
On 2/4/22 00:35, David Hildenbrand wrote: >> I thought this was simple. :) > > It really bugs me that it's under-specified what's supposed to happen > when the length is not aligned. > > BUT: in the posix world, "calling posix_madvise() shall not affect the > semantics of access to memory in the specified range". So we don't care > too much about if we align up/down, because it wouldn't affect the > semantics. Especially for MADV_DONTNEED/MADV_REMOVE as implemented by > Linux this is certainly different and the alignment handling matters. > > So I guess especially for MADV_DONTNEED/MADV_REMOVE we need a clear > specification what's supposed to happen if the length falls into the > middle of a huge page. We should document alignment handling for > madvise() in general I assume. > > IMHO we should have bailed out right from the start whenever something > is not properly aligned, but that ship has sailed. So I agree, maybe we > can make at least hugetlb MADV_DONTNEED obey the same (weird) rules as > ordinary pages. > > So b) would mean, requiring start to be hugepage aligned and aligning-up > the end. Still feels wrong but at least matches existing semantics. > > Hugetlb MADV_REMOVE semantics are unfortunate and we should document the > exception.
Thank you for all your comments David!
So, my plan was to make MADV_DONTNEED behave as described above: - EINVAL if start address not huge page size aligned - Align end/length up to huge page size.
The code I had for this was very specific to MADV_DONTNEED. I then thought, why not do the same for MADV_REMOVE as well? Or even more general, add this check and alignment to the vma parsing code in madvise.
It was then that I realized there are several madvise behaviors that take non-huge page size aligned addresses for hugetlb mappings today. Making huge page size alignment a requirement for all madvise behaviors could break existing code. So, it seems like it could only be added to MADV_DONTNEED as this functionality does not exist today. We then end up with MADV_DONTNEED as the only behavior requiring huge page size alignment for hugetlb mappings. Sigh!!!
I am now rethinking the decision to proceed with b) as described above.
With the exception of MADV_REMOVE (which we may be able to change for hugetlb), madvise operations operate on huge page size pages for hugetlb mappings. If start address is in the middle of a hugetlb page, we essentially align down to the beginning of the hugetlb page. If length lands in the middle of a hugetlb page, we essentially round up.
When adding MADV_REMOVE perhaps we should go with this align down start and align up end strategy that is used everywhere else? I really wish we could go back and change things, but as you know it is too late for that. -- Mike Kravetz
| |