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

["Garg, Shivank" <shivankg@amd.com>]

On Thu, 2026-09-10 at 12:03 +0200, David Hildenbrand (Arm) wrote:
> 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?

Compaction would work here because migration and compaction now checks the
mapping_unmovable(), while the Truncation still checks
mapping_inaccessible() to avoid writes to confidential memory.

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

yes, I'll do this.

Thanks,
Shivank