Re: [PATCH] mm: madvise(MADV_DODUMP) allow hugetlbfs pages

[Mike Kravetz <mike.kravetz@oracle.com>]

On 9/29/18 10:46 PM, Daniel Black wrote:
<snip>
> 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")
> 
> Fixes: 0103bd16fb90 ("mm: prepare VM_DONTDUMP for using in drivers")
> Reported-by: Kenneth Penza <kpenza@gmail.com>
> Signed-off-by: Daniel Black <daniel@linux.ibm.com>
> Buglink: https://lists.launchpad.net/maria-discuss/msg05245.html
> Cc: linux-mm@kvack.org
> Cc: Mike Kravetz <mike.kravetz@oracle.com>
> Cc: Konstantin Khlebnikov <khlebnikov@openvz.org>
> ---
>  mm/madvise.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 972a9eaa898b..71d21df2a3f3 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -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.

-- 
Mike Kravetz


>  			error = -EINVAL;
>  			goto out;
>  		}
>