linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (Oracle)" <ljs@kernel.org>
To: Anthony Yznaga <anthony.yznaga@oracle.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	 akpm@linux-foundation.org, david@kernel.org,
	Liam.Howlett@oracle.com, vbabka@kernel.org,  rppt@kernel.org,
	surenb@google.com, mhocko@suse.com, jannh@google.com,
	 pfalcato@suse.de, Jason@zx2c4.com
Subject: Re: [PATCH] mm: prevent droppable mappings from being locked
Date: Mon, 9 Mar 2026 14:28:08 +0000	[thread overview]
Message-ID: <53d66564-0596-45aa-b639-e2ea15356ae9@lucifer.local> (raw)
In-Reply-To: <20260306204550.8405-1-anthony.yznaga@oracle.com>

-cc old mail (this is going to take some time to propagate I realise :P)

On Fri, Mar 06, 2026 at 12:45:50PM -0800, Anthony Yznaga wrote:
> Mappings created with MAP_DROPPABLE cannot be locked via mlock() due
> to the check in mlock_fixup(). However, they will be locked indirectly
> if they are created after mlockall(MCL_FUTURE).

You need to add more details here.

For e.g.: 'in apply_mlockall_flags(), if the flags parameter has MCL_FUTURE set,
the current task's mm's default VMA flag field mm->def_flags has VM_LOCKED
applied to it. Therefore, in __mmap_complete(), extend the test for VM_SPECIAL
to include a test for VM_DROPPABLE'.

Do you have a test that can check for this? It'd be good to have a regression
test to assert that it now behaves correctly.

You could extend either tools/testing/selftests/mm/mlock2-tests.c or
droppable.c?

It's worth mentioning that mlockall(MCL_ONFAULT) is handled too, as
VM_LOCKONFAULT is always set with VM_LOCKED (the only difference being that,
when trying to fault in memory for VM_LOCKED ranges, gup exits early in
populate_vma_page_range() which has an explicit test for VM_LOCKONFAULT) , and
apply_mlockall_flags() will invoke mlock_fixup() which already has the
VM_DROPPABLE check.

>
> Fixes: 9651fcedf7b9 ("mm: add MAP_DROPPABLE for designating always lazily freeable mappings")

Do we want to cc: stable here?

> Signed-off-by: Anthony Yznaga <anthony.yznaga@oracle.com>
> ---
>  include/linux/mm.h | 3 +++
>  mm/mlock.c         | 4 ++--
>  mm/vma.c           | 2 +-
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5be3d8a8f806..bb830574d112 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -574,6 +574,9 @@ enum {
>  /* This mask represents all the VMA flag bits used by mlock */
>  #define VM_LOCKED_MASK	(VM_LOCKED | VM_LOCKONFAULT)
>
> +/* This mask prevents VMAs from being mlock'd */
> +#define VM_NO_MLOCK_MASK	(VM_SPECIAL | VM_DROPPABLE)
> +

It'd be preferable to not use the legacy VMA flags implementation, but if we're
backporting I guess... However there's only one place you need to update, the
other already manually checks droppable, and it'd make my life easier for the
VMA flags conversions to not define a flag like this also :)

>  /* These flags can be updated atomically via VMA/mmap read lock. */
>  #define VM_ATOMIC_SET_ALLOWED VM_MAYBE_GUARD
>
> diff --git a/mm/mlock.c b/mm/mlock.c
> index 2f699c3497a5..fd35c1e88c4c 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -472,9 +472,9 @@ static int mlock_fixup(struct vma_iterator *vmi, struct vm_area_struct *vma,
>  	int ret = 0;
>  	vm_flags_t oldflags = vma->vm_flags;
>
> -	if (newflags == oldflags || (oldflags & VM_SPECIAL) ||
> +	if (newflags == oldflags || (oldflags & VM_NO_MLOCK_MASK) ||
>  	    is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm) ||
> -	    vma_is_dax(vma) || vma_is_secretmem(vma) || (oldflags & VM_DROPPABLE))
> +	    vma_is_dax(vma) || vma_is_secretmem(vma))

This obviously wouldn't be necessary without adding a new VM_xxx...

>  		/* don't set VM_LOCKED or VM_LOCKONFAULT and don't count */
>  		goto out;
>
> diff --git a/mm/vma.c b/mm/vma.c
> index be64f781a3aa..1334622e4a03 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -2589,7 +2589,7 @@ static void __mmap_complete(struct mmap_state *map, struct vm_area_struct *vma)
>
>  	vm_stat_account(mm, vma->vm_flags, map->pglen);
>  	if (vm_flags & VM_LOCKED) {
> -		if ((vm_flags & VM_SPECIAL) || vma_is_dax(vma) ||
> +		if ((vm_flags & VM_NO_MLOCK_MASK) || vma_is_dax(vma) ||

For backport maybe just put an additional vm_flags & VM_DROPPABLE here?

>  					is_vm_hugetlb_page(vma) ||
>  					vma == get_gate_vma(mm))
>  			vm_flags_clear(vma, VM_LOCKED_MASK);
> --
> 2.47.3
>

Though I saw David suggested something different so that also addresses my review here :)

Cheers, Lorenzo


  parent reply	other threads:[~2026-03-09 14:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-06 20:45 Anthony Yznaga
2026-03-09 14:15 ` David Hildenbrand (Arm)
2026-03-09 14:31   ` Lorenzo Stoakes (Oracle)
2026-03-09 15:55     ` anthony.yznaga
2026-03-09 15:39   ` anthony.yznaga
2026-03-10  2:04   ` anthony.yznaga
2026-03-10  8:25     ` David Hildenbrand (Arm)
2026-03-09 14:28 ` Lorenzo Stoakes (Oracle) [this message]
2026-03-09 15:54   ` anthony.yznaga

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53d66564-0596-45aa-b639-e2ea15356ae9@lucifer.local \
    --to=ljs@kernel.org \
    --cc=Jason@zx2c4.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=anthony.yznaga@oracle.com \
    --cc=david@kernel.org \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=pfalcato@suse.de \
    --cc=rppt@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox