Re: [PATCH v3 2/9] mm: split AS_UNMOVABLE back out of AS_INACCESSIBLE

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

On 8/5/26 08:40, Shivank Garg wrote:
> Commit 27e6a24a4cf3 ("mm, virt: merge AS_UNMOVABLE and AS_INACCESSIBLE")
> folded the two flags into one, on the grounds that guest_memfd was the
> only user and always set both. But the two flags were added for
> different reasons and guard different things:
> 
>   AS_UNMOVABLE (0003e2a41468) marks a mapping whose folios cannot be
>   migrated.
> 
>   AS_INACCESSIBLE (c72ceafbd12c) marks a mapping whose contents must
>   not be directly R/W accessed. Its only job is to stop
>   truncate_inode_partial_folio() from zeroing the folio.
> 
> The merge assumed unmovable and inaccessible were the same thing.
> This cannot express a mapping that is inaccessible yet still movable,
> which is exactly what guest_memfd wants.
> 
> Reintroduce AS_UNMOVABLE and restore the original split: truncate keeps
> checking AS_INACCESSIBLE, while migration and compaction go back to
> checking AS_UNMOVABLE.
> 
> Currently guest_memfd sets both, so the resulting flags and behaviour
> are unchanged. Preparatory change to support folio migration for
> non-confidential guest_memfd VMs.
> 
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  include/linux/pagemap.h | 24 ++++++++++++++++++++----
>  mm/compaction.c         | 12 ++++++------
>  mm/migrate.c            |  2 +-
>  virt/kvm/guest_memfd.c  |  1 +
>  4 files changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 2c3718d592d6..a7dcaa66e4e3 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -210,6 +210,7 @@ enum mapping_flags {
>  	AS_WRITEBACK_MAY_DEADLOCK_ON_RECLAIM = 9,
>  	AS_KERNEL_FILE = 10,	/* mapping for a fake kernel file that shouldn't
>  				   account usage to user cgroups */
> +	AS_UNMOVABLE = 11,	/* The mapping cannot be moved, ever */
>  	/* Bits 16-25 are used for FOLIO_ORDER */
>  	AS_FOLIO_ORDER_BITS = 5,
>  	AS_FOLIO_ORDER_MIN = 16,
> @@ -322,11 +323,10 @@ static inline void mapping_clear_stable_writes(struct address_space *mapping)
>  static inline void mapping_set_inaccessible(struct address_space *mapping)
>  {
>  	/*
> -	 * It's expected inaccessible mappings are also unevictable. Compaction
> -	 * migrate scanner (isolate_migratepages_block()) relies on this to
> -	 * reduce page locking.
> +	 * The mapping's contents must not be accessed by the CPU through
> +	 * the kernel direct map or other internal paths (e.g. zeroing of
> +	 * pages during truncation).
>  	 */
> -	set_bit(AS_UNEVICTABLE, &mapping->flags);
>  	set_bit(AS_INACCESSIBLE, &mapping->flags);
>  }
>  
> @@ -335,6 +335,22 @@ static inline bool mapping_inaccessible(const struct address_space *mapping)
>  	return test_bit(AS_INACCESSIBLE, &mapping->flags);
>  }
>  
> +static inline void mapping_set_unmovable(struct address_space *mapping)
> +{
> +	/*
> +	 * It's expected unmovable mappings are also unevictable. Compaction
> +	 * migrate scanner (isolate_migratepages_block()) relies on this to
> +	 * reduce page locking.
> +	 */
> +	set_bit(AS_UNEVICTABLE, &mapping->flags);
> +	set_bit(AS_UNMOVABLE, &mapping->flags);
> +}
> +
> +static inline bool mapping_unmovable(const struct address_space *mapping)
> +{
> +	return test_bit(AS_UNMOVABLE, &mapping->flags);
> +}
> +
>  static inline void mapping_set_writeback_may_deadlock_on_reclaim(struct address_space *mapping)
>  {
>  	set_bit(AS_WRITEBACK_MAY_DEADLOCK_ON_RECLAIM, &mapping->flags);
> diff --git a/mm/compaction.c b/mm/compaction.c
> index f08765ade014..e6b0fdfaf79d 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1133,22 +1133,22 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>  		if (((mode & ISOLATE_ASYNC_MIGRATE) && is_dirty) ||
>  		    (mapping && is_unevictable)) {
>  			bool migrate_dirty = true;
> -			bool is_inaccessible;
> +			bool is_unmovable;
>  
>  			/*
>  			 * Only folios without mappings or that have
>  			 * a ->migrate_folio callback are possible to migrate
>  			 * without blocking.
>  			 *
> -			 * Folios from inaccessible mappings are not migratable.
> +			 * Folios from unmovable mappings are not migratable.
>  			 *
>  			 * However, we can be racing with truncation, which can
>  			 * free the mapping that we need to check. Truncation
>  			 * holds the folio lock until after the folio is removed
>  			 * from the page so holding it ourselves is sufficient.
>  			 *
> -			 * To avoid locking the folio just to check inaccessible,
> -			 * assume every inaccessible folio is also unevictable,
> +			 * To avoid locking the folio just to check unmovable,
> +			 * assume every unmovable folio is also unevictable,
>  			 * which is a cheaper test.  If our assumption goes
>  			 * wrong, it's not a correctness bug, just potentially
>  			 * wasted cycles.
> @@ -1161,9 +1161,9 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>  				migrate_dirty = !mapping ||
>  						mapping->a_ops->migrate_folio;
>  			}
> -			is_inaccessible = mapping && mapping_inaccessible(mapping);
> +			is_unmovable = mapping && mapping_unmovable(mapping);
>  			folio_unlock(folio);
> -			if (!migrate_dirty || is_inaccessible)
> +			if (!migrate_dirty || is_unmovable)
>  				goto isolate_fail_put;
>  		}
>  
> diff --git a/mm/migrate.c b/mm/migrate.c
> index dd15a84b2a52..d4dcd7f142ce 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1101,7 +1101,7 @@ static int move_to_new_folio(struct folio *dst, struct folio *src,
>  
>  	if (!mapping)
>  		rc = migrate_folio(mapping, dst, src, mode);
> -	else if (mapping_inaccessible(mapping))
> +	else if (mapping_unmovable(mapping))
>  		rc = -EOPNOTSUPP;
>  	else if (mapping->a_ops->migrate_folio)
>  		/*
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 45cbdf4801ec..169f75f95433 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -593,6 +593,7 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
>  	inode->i_size = size;
>  	mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
>  	mapping_set_inaccessible(inode->i_mapping);
> +	mapping_set_unmovable(inode->i_mapping);

For shared-only guest_memfd, is there even a reason to mark it as
mapping_set_inaccessible() ?

mapping_inaccessible() is only used in truncation and compaction logic.

Wouldn't we want compaction to work here?

IOW, for shared-only with migration support, can't we just not do
mapping_set_inaccessible() ?

-- 
Cheers,

David