Re: [PATCH v7 1/8] mm: Remove special swap entry functions
On Fri, Mar 26, 2021 at 11:07:58AM +1100, Alistair Popple wrote:
> Remove multiple similar inline functions for dealing with different
> types of special swap entries.
>
> Both migration and device private swap entries use the swap offset to
> store a pfn. Instead of multiple inline functions to obtain a struct
> page for each swap entry type use a common function
> pfn_swap_entry_to_page(). Also open-code the various entry_to_pfn()
> functions as this results is shorter code that is easier to understand.
>
> Signed-off-by: Alistair Popple
> Reviewed-by: Ralph Campbell
> Reviewed-by: Christoph Hellwig
>
> ---
>
> v7:
> * Reworded commit message to include pfn_swap_entry_to_page()
> * Added Christoph's Reviewed-by
>
> v6:
> * Removed redundant compound_page() call from inside PageLocked()
> * Fixed a minor build issue for s390 reported by kernel test bot
>
> v4:
> * Added pfn_swap_entry_to_page()
> * Reinstated check that migration entries point to locked pages
> * Removed #define swapcache_prepare which isn't needed for CONFIG_SWAP=0
> builds
> ---
> arch/s390/mm/pgtable.c | 2 +-
> fs/proc/task_mmu.c | 23 +-
> include/linux/swap.h| 4 +--
> include/linux/swapops.h | 69 ++---
> mm/hmm.c| 5 ++-
> mm/huge_memory.c| 4 +--
> mm/memcontrol.c | 2 +-
> mm/memory.c | 10 +++---
> mm/migrate.c| 6 ++--
> mm/page_vma_mapped.c| 6 ++--
> 10 files changed, 50 insertions(+), 81 deletions(-)
Looks good
Reviewed-by: Jason Gunthorpe
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 943cb2ba4442..3b2dda71d0ed 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -214,7 +214,7 @@ static inline bool hmm_is_device_private_entry(struct
> hmm_range *range,
> swp_entry_t entry)
> {
> return is_device_private_entry(entry) &&
> - device_private_entry_to_page(entry)->pgmap->owner ==
> + pfn_swap_entry_to_page(entry)->pgmap->owner ==
> range->dev_private_owner;
> }
>
> @@ -257,8 +257,7 @@ static int hmm_vma_handle_pte(struct mm_walk *walk,
> unsigned long addr,
> cpu_flags = HMM_PFN_VALID;
> if (is_write_device_private_entry(entry))
> cpu_flags |= HMM_PFN_WRITE;
> - *hmm_pfn = device_private_entry_to_pfn(entry) |
> - cpu_flags;
> + *hmm_pfn = swp_offset(entry) | cpu_flags;
Though swp_offset() seems poor here
Something like this seems nicer, maybe as an additional patch in this
series?
diff --git a/mm/hmm.c b/mm/hmm.c
index 943cb2ba444232..c06cbc4e3981b7 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -210,14 +210,6 @@ int hmm_vma_handle_pmd(struct mm_walk *walk, unsigned long
addr,
unsigned long end, unsigned long hmm_pfns[], pmd_t pmd);
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
-static inline bool hmm_is_device_private_entry(struct hmm_range *range,
- swp_entry_t entry)
-{
- return is_device_private_entry(entry) &&
- device_private_entry_to_page(entry)->pgmap->owner ==
- range->dev_private_owner;
-}
-
static inline unsigned long pte_to_hmm_pfn_flags(struct hmm_range *range,
pte_t pte)
{
@@ -226,6 +218,32 @@ static inline unsigned long pte_to_hmm_pfn_flags(struct
hmm_range *range,
return pte_write(pte) ? (HMM_PFN_VALID | HMM_PFN_WRITE) : HMM_PFN_VALID;
}
+static bool hmm_pte_handle_device_private(struct hmm_range *range, pte_t pte,
+ unsigned long *hmm_pfn)
+{
+ swp_entry_t entry = pte_to_swp_entry(pte);
+ struct page *device_page;
+ unsigned long cpu_flags;
+
+ if (is_device_private_entry(entry))
+ return false;
+
+ /*
+* If the device private page matches the device the caller understands
+* then return the private pfn directly. The caller must know what to do
+* with it.
+*/
+ device_page = pfn_swap_entry_to_page(entry);
+ if (device_page->pgmap->owner != range->dev_private_owner)
+ return false;
+
+ cpu_flags = HMM_PFN_VALID;
+ if (is_write_device_private_entry(entry))
+ cpu_flags |= HMM_PFN_WRITE;
+ *hmm_pfn = page_to_pfn(device_page) | cpu_flags;
+ return true;
+}
+
static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,
unsigned long end, pmd_t *pmdp, pte_t *ptep,
unsigned long *hmm_pfn)
@@ -247,20 +265,8 @@ static int hmm_vma_handle_pte(struct mm_walk *walk,
unsigned long addr,
}
if (!pte_present(pte)) {
- swp_entry_t entry = pte_to_swp_entry(pte);
-
- /*
-* Never fault in device private pages, but just report
-* the PFN even if not
[PATCH v7 1/8] mm: Remove special swap entry functions
Remove multiple similar inline functions for dealing with different
types of special swap entries.
Both migration and device private swap entries use the swap offset to
store a pfn. Instead of multiple inline functions to obtain a struct
page for each swap entry type use a common function
pfn_swap_entry_to_page(). Also open-code the various entry_to_pfn()
functions as this results is shorter code that is easier to understand.
Signed-off-by: Alistair Popple
Reviewed-by: Ralph Campbell
Reviewed-by: Christoph Hellwig
---
v7:
* Reworded commit message to include pfn_swap_entry_to_page()
* Added Christoph's Reviewed-by
v6:
* Removed redundant compound_page() call from inside PageLocked()
* Fixed a minor build issue for s390 reported by kernel test bot
v4:
* Added pfn_swap_entry_to_page()
* Reinstated check that migration entries point to locked pages
* Removed #define swapcache_prepare which isn't needed for CONFIG_SWAP=0
builds
---
arch/s390/mm/pgtable.c | 2 +-
fs/proc/task_mmu.c | 23 +-
include/linux/swap.h| 4 +--
include/linux/swapops.h | 69 ++---
mm/hmm.c| 5 ++-
mm/huge_memory.c| 4 +--
mm/memcontrol.c | 2 +-
mm/memory.c | 10 +++---
mm/migrate.c| 6 ++--
mm/page_vma_mapped.c| 6 ++--
10 files changed, 50 insertions(+), 81 deletions(-)
diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
index 18205f851c24..eec3a9d7176e 100644
--- a/arch/s390/mm/pgtable.c
+++ b/arch/s390/mm/pgtable.c
@@ -691,7 +691,7 @@ static void ptep_zap_swap_entry(struct mm_struct *mm,
swp_entry_t entry)
if (!non_swap_entry(entry))
dec_mm_counter(mm, MM_SWAPENTS);
else if (is_migration_entry(entry)) {
- struct page *page = migration_entry_to_page(entry);
+ struct page *page = pfn_swap_entry_to_page(entry);
dec_mm_counter(mm, mm_counter(page));
}
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 3cec6fbef725..08ee59d945c0 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -514,10 +514,8 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
} else {
mss->swap_pss += (u64)PAGE_SIZE << PSS_SHIFT;
}
- } else if (is_migration_entry(swpent))
- page = migration_entry_to_page(swpent);
- else if (is_device_private_entry(swpent))
- page = device_private_entry_to_page(swpent);
+ } else if (is_pfn_swap_entry(swpent))
+ page = pfn_swap_entry_to_page(swpent);
} else if (unlikely(IS_ENABLED(CONFIG_SHMEM) && mss->check_shmem_swap
&& pte_none(*pte))) {
page = xa_load(>vm_file->f_mapping->i_pages,
@@ -549,7 +547,7 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
swp_entry_t entry = pmd_to_swp_entry(*pmd);
if (is_migration_entry(entry))
- page = migration_entry_to_page(entry);
+ page = pfn_swap_entry_to_page(entry);
}
if (IS_ERR_OR_NULL(page))
return;
@@ -691,10 +689,8 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long
hmask,
} else if (is_swap_pte(*pte)) {
swp_entry_t swpent = pte_to_swp_entry(*pte);
- if (is_migration_entry(swpent))
- page = migration_entry_to_page(swpent);
- else if (is_device_private_entry(swpent))
- page = device_private_entry_to_page(swpent);
+ if (is_pfn_swap_entry(swpent))
+ page = pfn_swap_entry_to_page(swpent);
}
if (page) {
int mapcount = page_mapcount(page);
@@ -1383,11 +1379,8 @@ static pagemap_entry_t pte_to_pagemap_entry(struct
pagemapread *pm,
frame = swp_type(entry) |
(swp_offset(entry) << MAX_SWAPFILES_SHIFT);
flags |= PM_SWAP;
- if (is_migration_entry(entry))
- page = migration_entry_to_page(entry);
-
- if (is_device_private_entry(entry))
- page = device_private_entry_to_page(entry);
+ if (is_pfn_swap_entry(entry))
+ page = pfn_swap_entry_to_page(entry);
}
if (page && !PageAnon(page))
@@ -1444,7 +1437,7 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned long
addr, unsigned long end,
if (pmd_swp_soft_dirty(pmd))
flags |= PM_SOFT_DIRTY;
VM_BUG_ON(!is_pmd_migration_entry(pmd));
- page = migration_entry_to_page(entry);
+ page = pfn_swap_entry_to_page(entry);
}
#endif
