Re: [PATCH v6 06/13] mm: Add helpers to create migration entries from struct pages

[David Hildenbrand (Arm)]

On 2/2/26 12:36, Jordan Niethe wrote:
> To create a new migration entry for a given struct page, that page is
> first converted to its pfn, before passing the pfn to
> make_readable_migration_entry() (and friends).
> 
> A future change will remove device private pages from the physical
> address space. This will mean that device private pages no longer have a
> pfn and must be handled separately.
> 
> Prepare for this with a new set of helpers:
> 
>   - make_readable_migration_entry_from_page()
>   - make_readable_exclusive_migration_entry_from_page()
>   - make_writable_migration_entry_from_page()

This is pretty horrible. And everything I see in successive patches
around that is not any better.

Seriously, look at how terrible
make_readable_exclusive_migration_entry_from_page() is.
make_readable_exclusive_migration_device_private_entry is not any
better, but at least doesn't have this questionable flags parameter.

Correct me if I am wrong: all we want is:

make_readable_migration_entry() etc. to create something that
softleaf_to_page() / softleaf_to_folio() can properly convert to a
folio, right?

So maybe we should convert make_readable_migration_entry() and friends
to consume a page (not add new functions ...), and let it internally
figure out how to communicate in the entry that what we are dealing with
is not an ordinary pfn, but a special device-private entry.


> 
> Signed-off-by: Jordan Niethe <jniethe@xxxxxxxxxx>
> ---
> v1:
>   - New to series
> v2:
>   - Add flags param
> v3:
>   - No change
> v4:
>   - s/make_writeable_migration_entry_from_page/make_writable_migration_entry_from_page/
>     for the !CONFIG_MIGRATION case
> ---
>  include/linux/leafops.h | 14 ++++++++++++++
>  include/linux/swapops.h | 34 ++++++++++++++++++++++++++++++++++
>  mm/huge_memory.c        | 29 +++++++++++++++++------------
>  mm/hugetlb.c            | 15 +++++++++------
>  mm/memory.c             |  5 +++--
>  mm/migrate_device.c     | 12 ++++++------
>  mm/mprotect.c           | 10 +++++++---
>  mm/rmap.c               | 12 ++++++------
>  8 files changed, 96 insertions(+), 35 deletions(-)
> 
> diff --git a/include/linux/leafops.h b/include/linux/leafops.h
> index a9ff94b744f2..52a1af3eb954 100644
> --- a/include/linux/leafops.h
> +++ b/include/linux/leafops.h
> @@ -363,6 +363,20 @@ static inline unsigned long softleaf_to_pfn(softleaf_t entry)
>  	return swp_offset(entry) & SWP_PFN_MASK;
>  }
>  
> +/**
> + * softleaf_to_flags() - Obtain flags encoded within leaf entry.
> + * @entry: Leaf entry, softleaf_has_pfn(@entry) must return true.
> + *
> + * Returns: The flags associated with the leaf entry.
> + */
> +static inline unsigned long softleaf_to_flags(softleaf_t entry)
> +{
> +	VM_WARN_ON_ONCE(!softleaf_has_pfn(entry));
> +
> +	/* Temporary until swp_entry_t eliminated. */
> +	return swp_offset(entry) & (SWP_MIG_YOUNG | SWP_MIG_DIRTY);
> +}

The whole flags stuff does not really belong in this patch. If there are
cleanups to be had, perform them separately, please.


-- 
Cheers,

David