Skip to contentSkip to conversation

mprotect and hugetlb mappings

DISCUSSION Mike Kravetz 4 participants 6 messages Jul 5, 2023 – Jul 17, 2023 Memory management

lore
Share & tools
lore
#0 lore raw

I was recently asked about the behavior of mprotect on a hugetlb mapping where addr or addr+len is not hugetlb page size aligned. As one might expect, EINVAL is returned in such cases. However, the man page makes no mention of alignment requirements for hugetlb mappings.

I am happy to submit man page updates if people agree this is the correct behavior. We might even want to check alignment earlier in the code path as we fail when trying to split the vma today.

An alternative behavior would be to operate on whole hugetlb pages within the range addr - addr+len.

Signature · 1 line
Mike Kravetz
#1 lore raw
Quoted 11 lines · Mike Kravetz

I was recently asked about the behavior of mprotect on a hugetlb mapping where addr or addr+len is not hugetlb page size aligned. As one might expect, EINVAL is returned in such cases. However, the man page makes no mention of alignment requirements for hugetlb mappings.

I am happy to submit man page updates if people agree this is the correct behavior. We might even want to check alignment earlier in the code path as we fail when trying to split the vma today.

An alternative behavior would be to operate on whole hugetlb pages within the range addr - addr+len.

After a careful re-reading of the mprotect() man page, I suggest the following behaviour ...

addr must be a multiple of the hpage size. Otherwise -EINVAL. len should be rounded up to hpage size.

I wonder how likely this change would be to break userspace code. Maybe some test cases.

#2 lore raw
Quoted 13 lines · Matthew Wilcox
Quoted 11 lines · Mike Kravetz

I was recently asked about the behavior of mprotect on a hugetlb mapping where addr or addr+len is not hugetlb page size aligned. As one might expect, EINVAL is returned in such cases. However, the man page makes no mention of alignment requirements for hugetlb mappings.

I am happy to submit man page updates if people agree this is the correct behavior. We might even want to check alignment earlier in the code path as we fail when trying to split the vma today.

An alternative behavior would be to operate on whole hugetlb pages within the range addr - addr+len.

Quoted 8 lines

After a careful re-reading of the mprotect() man page, I suggest the following behaviour ...

addr must be a multiple of the hpage size. Otherwise -EINVAL. len should be rounded up to hpage size.

I wonder how likely this change would be to break userspace code. Maybe some test cases.

My concern is that this is the approach I took with huegtlb MADV_DONTNEED, and this caused problems discussed and eventually modified here:
https://lore.kernel.org/linux-mm/[email protected]/

In the MADV_DONTNEED case we were throwing away data. With mprotect we are only modifying access to data.

Signature · 1 line
Mike Kravetz
#3 lore raw
Quoted 23 lines · Mike Kravetz
Quoted 13 lines · Matthew Wilcox
Quoted 11 lines · Mike Kravetz

I was recently asked about the behavior of mprotect on a hugetlb mapping where addr or addr+len is not hugetlb page size aligned. As one might expect, EINVAL is returned in such cases. However, the man page makes no mention of alignment requirements for hugetlb mappings.

I am happy to submit man page updates if people agree this is the correct behavior. We might even want to check alignment earlier in the code path as we fail when trying to split the vma today.

An alternative behavior would be to operate on whole hugetlb pages within the range addr - addr+len.

Quoted 8 lines

After a careful re-reading of the mprotect() man page, I suggest the following behaviour ...

addr must be a multiple of the hpage size. Otherwise -EINVAL. len should be rounded up to hpage size.

I wonder how likely this change would be to break userspace code. Maybe some test cases.

Quoted 6 lines

My concern is that this is the approach I took with huegtlb MADV_DONTNEED, and this caused problems discussed and eventually modified here:
https://lore.kernel.org/linux-mm/[email protected]/

In the MADV_DONTNEED case we were throwing away data. With mprotect we are only modifying access to data.

That can still confuse some userspace, no? I think realistically we can only document the current implementation better, maybe improve it without changing observed behavior as you suggested wrt the split vma fail. But changing it would be dangerous.

#4 lore raw
Quoted 28 lines · Vlastimil Babka
Quoted 27 lines · Mike Kravetz
Quoted 13 lines · Matthew Wilcox
Quoted 11 lines · Mike Kravetz

I was recently asked about the behavior of mprotect on a hugetlb mapping where addr or addr+len is not hugetlb page size aligned. As one might expect, EINVAL is returned in such cases. However, the man page makes no mention of alignment requirements for hugetlb mappings.

I am happy to submit man page updates if people agree this is the correct behavior. We might even want to check alignment earlier in the code path as we fail when trying to split the vma today.

An alternative behavior would be to operate on whole hugetlb pages within the range addr - addr+len.

Quoted 8 lines

After a careful re-reading of the mprotect() man page, I suggest the following behaviour ...

addr must be a multiple of the hpage size. Otherwise -EINVAL. len should be rounded up to hpage size.

I wonder how likely this change would be to break userspace code. Maybe some test cases.

My concern is that this is the approach I took with huegtlb MADV_DONTNEED, and this caused problems discussed and eventually modified here:
https://lore.kernel.org/linux-mm/[email protected]/

Quoted 7 lines
Quoted 2 lines

In the MADV_DONTNEED case we were throwing away data. With mprotect we are only modifying access to data.

That can still confuse some userspace, no? I think realistically we can only document the current implementation better, maybe improve it without changing observed behavior as you suggested wrt the split vma fail. But changing it would be dangerous.

Thanks for the comments Vlastimil.

That would be my thought/preferred path forward. Simply document the current behavior, and MAYBE update code to be more explicit.

Any other thoughts?

Signature · 1 line
Mike Kravetz
#5 lore raw
Quoted 37 lines · Mike Kravetz
Quoted 28 lines · Vlastimil Babka
Quoted 27 lines · Mike Kravetz
Quoted 13 lines · Matthew Wilcox
Quoted 11 lines · Mike Kravetz

I was recently asked about the behavior of mprotect on a hugetlb mapping where addr or addr+len is not hugetlb page size aligned. As one might expect, EINVAL is returned in such cases. However, the man page makes no mention of alignment requirements for hugetlb mappings.

I am happy to submit man page updates if people agree this is the correct behavior. We might even want to check alignment earlier in the code path as we fail when trying to split the vma today.

An alternative behavior would be to operate on whole hugetlb pages within the range addr - addr+len.

Quoted 8 lines

After a careful re-reading of the mprotect() man page, I suggest the following behaviour ...

addr must be a multiple of the hpage size. Otherwise -EINVAL. len should be rounded up to hpage size.

I wonder how likely this change would be to break userspace code. Maybe some test cases.

My concern is that this is the approach I took with huegtlb MADV_DONTNEED, and this caused problems discussed and eventually modified here:
https://lore.kernel.org/linux-mm/[email protected]/

Quoted 7 lines
Quoted 2 lines

In the MADV_DONTNEED case we were throwing away data. With mprotect we are only modifying access to data.

That can still confuse some userspace, no? I think realistically we can only document the current implementation better, maybe improve it without changing observed behavior as you suggested wrt the split vma fail. But changing it would be dangerous.

Quoted 6 lines

Thanks for the comments Vlastimil.

That would be my thought/preferred path forward. Simply document the current behavior, and MAYBE update code to be more explicit.

Any other thoughts?

I agree.

Signature · 3 lines
Cheers,

David / dhildenb