Re: [PATCH 23/39] mm/mlock: eliminate weird VMA_IO_BIT abuse and simplify

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing munlock during concurrent unmap due to vma flag check regression
--

commit 2a2d17538653740326938f5e55760a431c727840
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

mm/mlock: eliminate weird VMA_IO_BIT abuse and simplify

This patch simplifies the mlock implementation by eliminating the use of
the VMA_IO_BIT flag. Instead, it temporarily sets VMA_LOCKONFAULT_BIT and
clears VMA_LOCKED_BIT during page table walks to prevent double counting.
It also updates various mlock functions to check the new VMA lock state.

> diff --git a/mm/internal.h b/mm/internal.h
> index 6e27d3b10c015..04b1f1d3d9604 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -993,7 +992,7 @@ static inline void munlock_vma_folio(struct folio *folio,
>  	 * always munlock the folio and page reclaim will correct it
>  	 * if it's wrong.
>  	 */
> -	if (unlikely(vma->vm_flags & VM_LOCKED))
> +	if (unlikely(vma_test(vma, VMA_LOCKED_BIT)))

[Severity: High]
Does this change leak the mlock_count if a concurrent unmap occurs?

If a process calls mlock(), mlock_vma_pages_range() now clears
VMA_LOCKED_BIT and sets VMA_LOCKONFAULT_BIT during the walk. If another
process concurrently unmaps a file-backed page in this VMA (for example,
via ftruncate() which only takes i_mmap_rwsem), the unmap path calls
folio_remove_rmap_pte() -> munlock_vma_folio().

Because VMA_LOCKED_BIT is temporarily clear during the mlock walk, this
check evaluates to false and skips calling munlock_folio(). This leaves
the folio pinned on the unevictable LRU with an elevated mlock_count.

Would it be better to check for VMA_LOCKED_MASK here, similar to how
try_to_unmap_one() was updated in this patch?

>  		munlock_folio(folio);
>  }
>  
> diff --git a/mm/mlock.c b/mm/mlock.c
> index 39215a3eab1fb..4235a1518fc9e 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c

[ ... ]

> @@ -428,31 +417,29 @@ static void mlock_vma_pages_range(struct vm_area_struct *vma,
>  		.pmd_entry = mlock_pte_range,
>  		.walk_lock = PGWALK_WRLOCK_VERIFY,
>  	};
> +	const bool lock = vma_flags_test(new_vma_flags, VMA_LOCKED_BIT);
> +	vma_flags_t walk_flags = *new_vma_flags;
>  
>  	/*
> -	 * There is a slight chance that concurrent page migration,
> -	 * or page reclaim finding a page of this now-VMA_LOCKED_BIT vma,
> -	 * will call mlock_vma_folio() and raise page's mlock_count:
> -	 * double counting, leaving the page unevictable indefinitely.
> -	 * Communicate this danger to mlock_vma_folio() with VMA_IO_BIT,
> -	 * which is a VMA_SPECIAL_FLAGS flag not allowed on VMA_LOCKED_BIT vmas.
> -	 * mmap_lock is held in write mode here, so this weird
> -	 * combination should not be visible to other mmap_lock users;
> -	 * but WRITE_ONCE so rmap walkers must see VMA_IO_BIT if VMA_LOCKED_BIT.
> +	 * LOCKONFAULT without LOCKED never otherwise occurs: it marks a walk in
> +	 * progress so that rmap-side callers, which test VMA_LOCKED_BIT, do not
> +	 * count folios, while try_to_unmap_one(), which tests VMA_LOCKED_MASK,
> +	 * still refuses to unmap them.
>  	 */
> -	if (vma_flags_test(new_vma_flags, VMA_LOCKED_BIT))
> -		vma_flags_set(new_vma_flags, VMA_IO_BIT);
> +	if (lock) {
> +		vma_flags_clear(&walk_flags, VMA_LOCKED_BIT);
> +		vma_flags_set(&walk_flags, VMA_LOCKONFAULT_BIT);
> +	}

[Severity: High]
Because VMA_LOCKED_BIT is temporarily cleared here, any concurrent rmap
walks (like a truncation unmapping pages) that rely on munlock_vma_folio()
will see a cleared lock bit and fail to drop the mlock_count.

> +
>  	vma_start_write(vma);
> -	vma_flags_reset_once(vma, new_vma_flags);
> +	vma_flags_reset_once(vma, &walk_flags);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce22b@kernel.org?part=23