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 <[email protected]>
> ---
> 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
