Re: [PATCH v7 1/8] mm: Remove special swap entry functions
From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2021-03-30 18:39:34
Also in:
dri-devel, linux-mm, lkml, nouveau
Subsystem:
hmm - heterogeneous memory management, memory management, the rest · Maintainers:
Jason Gunthorpe, Leon Romanovsky, Andrew Morton, Linus Torvalds
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 <apopple@nvidia.com> Reviewed-by: Ralph Campbell <redacted> Reviewed-by: Christoph Hellwig <hch@lst.de> --- 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 <jgg@nvidia.com>
quoted hunk ↗ jump to hunk
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 present. - */ - if (hmm_is_device_private_entry(range, entry)) { - 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; + if (hmm_pte_handle_device_private(range, pte, hmm_pfn)) return 0; - } required_fault = hmm_pte_need_fault(hmm_vma_walk, pfn_req_flags, 0);
Jason