mm: madvise(MADV_DODUMP) allow hugetlbfs pages
Subsystems (2)
PATCH 1/1 mm: madvise(MADV_DODUMP) allow hugetlbfs pages
Reproducer, assuming 2M of hugetlbfs available:
Hugetlbfs mounted, size=2M and option user=testuser
# mount | grep ^hugetlbfs hugetlbfs on /dev/hugepages type hugetlbfs (rw,pagesize=2M,user=dan) # sysctl vm.nr_hugepages=1 vm.nr_hugepages = 1 # grep Huge /proc/meminfo AnonHugePages: 0 kB ShmemHugePages: 0 kB HugePages_Total: 1 HugePages_Free: 1 HugePages_Rsvd: 0 HugePages_Surp: 0 Hugepagesize: 2048 kB Hugetlb: 2048 kB
Code:
#include <sys/mman.h>
#include <stddef.h>
#define SIZE 2*1024*1024
int main()
{
void *ptr;
ptr = mmap(NULL, SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_HUGETLB | MAP_ANONYMOUS, -1, 0);
madvise(ptr, SIZE, MADV_DONTDUMP);
madvise(ptr, SIZE, MADV_DODUMP);
}Compile and strace:
mmap(NULL, 2097152, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_HUGETLB, -1, 0) = 0x7ff7c9200000 madvise(0x7ff7c9200000, 2097152, MADV_DONTDUMP) = 0
madvise(0x7ff7c9200000, 2097152, MADV_DODUMP) = -1 EINVAL (Invalid argument)
hugetlbfs pages have VM_DONTEXPAND in the VmFlags driver pages based on author testing with analysis from Florian Weimer[1].
The inclusion of VM_DONTEXPAND into the VM_SPECIAL defination was a consequence of the large useage of VM_DONTEXPAND in device drivers.
A consequence of [2] is that VM_DONTEXPAND marked pages are unable to be marked DODUMP.
A user could quite legitimately madvise(MADV_DONTDUMP) their hugetlbfs memory for a while and later request that madvise(MADV_DODUMP) on the same memory. We correct this omission by allowing madvice(MADV_DODUMP) on hugetlbfs pages.
[1] https://stackoverflow.com/questions/52548260/madvisedodump-on-the-same-ptr-size-as-a-successful-madvisedontdump-fails-wit
[2] commit 0103bd16fb90 ("mm: prepare VM_DONTDUMP for using in drivers")
--- mm/madvise.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/madvise.c +1 -1
@@ -96,7 +96,7 @@ static long madvise_behavior(struct vm_area_struct *vma, new_flags |=VM_DONTDUMP ;break ;case MADV_DODUMP : -if (new_flags &VM_SPECIAL ) { +if (!is_vm_hugetlb_page (vma) && new_flags &VM_SPECIAL ) { error = -EINVAL ; goto out; }
Signature · 1 line
2.19.0
On 9/29/18 10:46 PM, Daniel Black wrote:
<snip>
Quoted 34 lines
hugetlbfs pages have
VM_DONTEXPANDin the VmFlags driver pages based on author testing with analysis from Florian Weimer[1].The inclusion of
VM_DONTEXPANDinto theVM_SPECIALdefination was a consequence of the large useage ofVM_DONTEXPANDin device drivers.A consequence of [2] is that
VM_DONTEXPANDmarked pages are unable to be marked DODUMP.A user could quite legitimately
madvise(MADV_DONTDUMP)their hugetlbfs memory for a while and later request thatmadvise(MADV_DODUMP)on the same memory. We correct this omission by allowingmadvice(MADV_DODUMP)on hugetlbfs pages.[1] https://stackoverflow.com/questions/52548260/madvisedodump-on-the-same-ptr-size-as-a-successful-madvisedontdump-fails-wit
[2] commit0103bd16fb90("mm: prepareVM_DONTDUMPfor using in drivers")Fixes 0103bd16fb90 ("mm: prepare VM_DONTDUMP for using in drivers")Reported-by Kenneth Penza <[email protected]>Signed-off-by Daniel Black <[email protected]>Cc Mike Kravetz <[email protected]>Cc Konstantin Khlebnikov <[email protected]>--- mm/madvise.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/madvise.c +0
-0
mm/madvise.c static long madvise_behavior(struct vm_area_struct *vma, · line 99
Quoted 5 lines
mm/madvise.c +1
-1@@ -96,7 +96,7 @@ static long madvise_behavior(struct vm_area_struct *vma, new_flags |=VM_DONTDUMP ;break ;case MADV_DODUMP : -if (new_flags &VM_SPECIAL ) { +if (!is_vm_hugetlb_page (vma) && new_flags &VM_SPECIAL ) {
Thanks Daniel,
This is certainly a regression. My only question is whether this condition should be more specific and test the default hugetlb vma flags (VM_DONTEXPAND | VM_HUGETLB). Or, whether simply checking VM_HUGETLB as you have done above is sufficient. Only reason for concern is that I am not 100% certain other VM_SPECIAL flags could not be set in VM_HUGETLB vma.
Perhaps Konstantin has an opinion as he did a bunch of the vm_flag reorg.
Signature · 1 line
Mike Kravetz
Quoted 4 lines
mm/madvise.c +0
-0error = -EINVAL ; goto out; }
Quoted 16 lines · On Mon, 1 Oct 2018 15:11:32 -0700 Mike Kravetz <[email protected]> wrote:
On 9/29/18 10:46 PM, Daniel Black wrote:
<snip>mm/madvise.c static long madvise_behavior(struct · line 100
Quoted 11 lines
mm/madvise.c +1
-1@@ -96,7 +96,7 @@ static long madvise_behavior(struct vm_area_struct *vma, new_flags |=VM_DONTDUMP ;break ;case MADV_DODUMP : -if (new_flags &VM_SPECIAL ) { +if (!is_vm_hugetlb_page (vma) && new_flags &VM_SPECIAL ) {Thanks Daniel,
Quoted 6 lines
This is certainly a regression. My only question is whether this condition should be more specific and test the default hugetlb vma flags (
VM_DONTEXPAND|VM_HUGETLB).Or, whether simply checking
VM_HUGETLBas you have done above is sufficient.
The is_vm_hugetlb_page() function seems widely used elsewhere for that single purpose.
Quoted 3 lines
Only reason for
concern is that I am not 100% certain otherVM_SPECIALflags could not be set inVM_HUGETLBvma.
They might be, but being a VM_HUGETLB flag is the main criteria for being able to madvise(DODUMP) on the memory. It highlight its user memory for the user to do as they wish.
When 314e51b9851b was added, it seemed the direction was to kill of the VM_RESERVED, now a few years later VM_SPECIAL seems to be replacing this. I think it would be better to preserve the original goal and keep flags having a single meaning.
The purpose in 0103bd16fb90 as I surmise it, is that VM_IO | VM_PFNMAP | VM_MIXEDMAP are the true things that want to be prevented from having madvise(DO_DUMP) on them, based on frequent use of DONT_DUMP with those memory pages. Was VM_DONTEXPAND an intentional inclusion there it did it just get included with VM_SPECIAL?
Either way, I've tried to keep to the principles of the is_vm_hugetlb_page function being the authoritative source of a HUGETLB page.
Quoted 3 lines
Perhaps Konstantin has an opinion as he did a bunch of the
vm_flagreorg.
Thanks for the review.
Mike Kravetz
Reviewed-byQuoted 52 lines · Daniel Black
Quoted 16 lines · On Mon, 1 Oct 2018 15:11:32 -0700 Mike Kravetz <[email protected]> wrote:
On 9/29/18 10:46 PM, Daniel Black wrote:
<snip>mm/madvise.c static long madvise_behavior(struct · line 100
Quoted 11 lines
mm/madvise.c +1
-1@@ -96,7 +96,7 @@ static long madvise_behavior(struct vm_area_struct *vma, new_flags |=VM_DONTDUMP ;break ;case MADV_DODUMP : -if (new_flags &VM_SPECIAL ) { +if (!is_vm_hugetlb_page (vma) && new_flags &VM_SPECIAL ) {Thanks Daniel,
Quoted 6 lines
This is certainly a regression. My only question is whether this condition should be more specific and test the default hugetlb vma flags (
VM_DONTEXPAND|VM_HUGETLB).Or, whether simply checking
VM_HUGETLBas you have done above is sufficient.The
is_vm_hugetlb_page()function seems widely used elsewhere for that single purpose.Quoted 3 lines
Only reason for
concern is that I am not 100% certain otherVM_SPECIALflags could not be set inVM_HUGETLBvma.They might be, but being a
VM_HUGETLBflag is the main criteria for being able to madvise(DODUMP) on the memory. It highlight its user memory for the user to do as they wish.When
314e51b9851bwas added, it seemed the direction was to kill of theVM_RESERVED, now a few years laterVM_SPECIALseems to be replacing this. I think it would be better to preserve the original goal and keep flags having a single meaning.The purpose in
0103bd16fb90as I surmise it, is thatVM_IO|VM_PFNMAP|VM_MIXEDMAPare the true things that want to be prevented from havingmadvise(DO_DUMP)on them, based on frequent use ofDONT_DUMPwith those memory pages. WasVM_DONTEXPANDan intentional inclusion there it did it just get included withVM_SPECIAL?Either way, I've tried to keep to the principles of the
is_vm_hugetlb_pagefunction being the authoritative source of a HUGETLB page.
Quoted 6 lines
Quoted 3 lines
Perhaps Konstantin has an opinion as he did a bunch of the
vm_flagreorg.Thanks for the review.
Thanks for the explanation. I was thinking of the case where hugetlb pages could be used for RDMA, and wanted to make sure those drivers were not doing anything to vm_flags. A quick look reveals nothing special happening.
You can add,
Adding Andrew on Cc:
Signature · 1 line
Mike Kravetz