Re: [PATCH v5 25/38] mm: remove VMA_IO_BIT check in vma[_flags]_is_mm_managed()

["David Hildenbrand (Arm)" <david@kernel.org>]

On 10/9/26 20:04, Lorenzo Stoakes (ARM) wrote:
> We have now made it such that every driver which sets VMA_IO_BIT marks it
> as not mm-managed.

Confusing given that mm-managed check tests VMA_IO_BIT.

Maybe combining this with the next paragraph could make this easier to grasp

"vma_flags_is_mm_managed() still checks for VMA_IO_BIT, and if set, treats it as
non-mm-managed. This was previously required to handle driver that would set
VMA_IO_BIT but none of the other flags that would indicate a VMA as non-mm-managed.

We already established now that a driver that sets VMA_IO_BIT must set one of
the other VMA flags that allow for identifying it as non-mm-managed.

...
"

> 
> However, vma_flags_is_mm_managed() currently treats VMA_IO_BIT as
> disqualifying. This was a product of drivers previously marking a range as
> not mm-managed by
> setting VMA_IO_BIT alone.
> 
> Fix this by removing the VMA_IO_BIT check in vma_flags_is_mm_managed(),
> and update mmap_validate_vma_flags() to use vma_flags_is_mm_managed()
> rather than open-coding the VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT check.
> 
> This change means that vma[_flags]_can_merge() doesn't check VMA_IO_BIT any
> longer (which is now redundant) as it calls vma_flags_is_mm_managed().
> 
> Now that the predicate means precisely VMA_PFNMAP_BIT or VMA_MIXEDMAP_BIT,
> also use it at the other sites which open-code that pair, so the intent is
> stated rather than the flags, with no functional change:
> 
> zap_special_vma_range() only zaps mappings which are not mm-managed, as
> drivers use it
> to tear down ranges they established themselves.
> 
> The mprotect() arch PFN modification check applies to mappings not managed
> by core mm, which may map PFNs without struct pages.
> 
> NUMA balancing skips VM_MIXEDMAP mappings having already excluded VM_IO
> and VM_PFNMAP mappings via vma_migratable(), so it skips exactly the
> mappings not managed by core mm - say so.
> 
> Finally, update the VMA userland merge 'special' flag tests to no longer
> assert that VMA_IO_BIT prevents merge as VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT
> now suffices.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
>  include/linux/mm.h              |  3 +--
>  kernel/sched/fair.c             |  2 +-
>  mm/memory.c                     |  7 ++++---
>  mm/mprotect.c                   |  3 +--
>  mm/vma.c                        |  2 +-
>  tools/testing/vma/include/dup.h |  3 +--
>  tools/testing/vma/tests/merge.c | 10 ++--------
>  7 files changed, 11 insertions(+), 19 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6b5f06cd174a..3ddb463db895 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1627,8 +1627,7 @@ static inline bool vma_is_shared_maywrite(const struct vm_area_struct *vma)
>   */
>  static inline bool vma_flags_is_mm_managed(const vma_flags_t *flags)
>  {
> -	return !vma_flags_test_any(flags, VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT,
> -				   VMA_IO_BIT);
> +	return !vma_flags_test_any(flags, VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT);
>  }
>  
>  /**
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 8dff37059faf..a07a658cd99f 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4212,7 +4212,7 @@ static void task_numa_work(struct callback_head *work)
>  
>  	for (; vma; vma = vma_next(&vmi)) {
>  		if (!vma_migratable(vma) || !vma_policy_mof(vma) ||
> -			is_vm_hugetlb_page(vma) || (vma->vm_flags & VM_MIXEDMAP)) {
> +			is_vm_hugetlb_page(vma) || !vma_is_mm_managed(vma)) {
>  			trace_sched_skip_vma_numa(mm, vma, NUMAB_SKIP_UNSUITABLE);
>  			continue;
>  		}
> diff --git a/mm/memory.c b/mm/memory.c
> index e5c1d1c35bc7..094983683f24 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2343,19 +2343,20 @@ void zap_vma_range(struct vm_area_struct *vma, unsigned long address,
>  }
>  
>  /**
> - * zap_special_vma_range - zap all page table entries in a special vma range
> + * zap_special_vma_range - zap all page table entries in a VMA which is not
> + * mm-managed
>   * @vma: the vma covering the range to zap
>   * @address: starting address of the range to zap
>   * @size: number of bytes to zap
>   *
>   * This function does nothing when the provided address range is not fully
> - * contained in @vma, or when the @vma is not VM_PFNMAP or VM_MIXEDMAP.
> + * contained in @vma, or when @vma is mm-managed.
>   */
>  void zap_special_vma_range(struct vm_area_struct *vma, unsigned long address,
>  		unsigned long size)
>  {
>  	if (!range_in_vma(vma, address, address + size) ||
> -	   !(vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP)))
> +	   vma_is_mm_managed(vma))
>  		return;
>  
>  	zap_vma_range(vma, address, size);
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 2888ee638d87..706c679d4281 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -783,8 +783,7 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
>  	 * uncommon case, so doesn't need to be very optimized.
>  	 */
>  	if (arch_has_pfn_modify_check() &&
> -	    vma_flags_test_any(&old_vma_flags, VMA_PFNMAP_BIT,
> -			       VMA_MIXEDMAP_BIT) &&
> +	    !vma_flags_is_mm_managed(&old_vma_flags) &&
>  	    !vma_flags_test_any_mask(&new_vma_flags, VMA_ACCESS_FLAGS)) {
>  		pgprot_t new_pgprot = vm_get_page_prot(newflags);
>  
> diff --git a/mm/vma.c b/mm/vma.c
> index 51bdbe08bb02..cb8729b195ea 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -2796,7 +2796,7 @@ static int mmap_validate_vma_flags(const vma_flags_t *flags)
>  		return -EINVAL;
>  #endif
>  
> -	if (!vma_flags_test_any(flags, VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT)) {
> +	if (vma_flags_is_mm_managed(flags)) {

Ah, there is the change.

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David