mm: remove the extra ZONE_DEVICE struct page refcount
authorChristoph Hellwig <hch@lst.de>
Wed, 16 Feb 2022 04:31:36 +0000 (15:31 +1100)
committerMatthew Wilcox (Oracle) <willy@infradead.org>
Thu, 3 Mar 2022 17:47:33 +0000 (12:47 -0500)
ZONE_DEVICE struct pages have an extra reference count that complicates
the code for put_page() and several places in the kernel that need to
check the reference count to see that a page is not being used (gup,
compaction, migration, etc.). Clean up the code so the reference count
doesn't need to be treated specially for ZONE_DEVICE pages.

Note that this excludes the special idle page wakeup for fsdax pages,
which still happens at refcount 1.  This is a separate issue and will
be sorted out later.  Given that only fsdax pages require the
notifiacation when the refcount hits 1 now, the PAGEMAP_OPS Kconfig
symbol can go away and be replaced with a FS_DAX check for this hook
in the put_page fastpath.

Based on an earlier patch from Ralph Campbell <rcampbell@nvidia.com>.

Link: https://lkml.kernel.org/r/20220210072828.2930359-8-hch@lst.de
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
Tested-by: "Sierra Guiza, Alejandro (Alex)" <alex.sierra@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Alistair Popple <apopple@nvidia.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Cc: Christian Knig <christian.koenig@amd.com>
Cc: Karol Herbst <kherbst@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: Muchun Song <songmuchun@bytedance.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
13 files changed:
arch/powerpc/kvm/book3s_hv_uvmem.c
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
drivers/gpu/drm/nouveau/nouveau_dmem.c
fs/Kconfig
include/linux/memremap.h
include/linux/mm.h
lib/test_hmm.c
mm/Kconfig
mm/internal.h
mm/memcontrol.c
mm/memremap.c
mm/migrate.c
mm/swap.c

index 881951604227a582934f19606debc96f803d32b2..8cabdb39cbbca58c902a9c441056c26c267c2173 100644 (file)
@@ -713,7 +713,6 @@ static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm)
 
        dpage = pfn_to_page(uvmem_pfn);
        dpage->zone_device_data = pvt;
-       get_page(dpage);
        lock_page(dpage);
        return dpage;
 out_clear:
index cb835f95a76e66902b97c2abb51a9730348f4110..e27ca375876230a5ecb661192bc2e98d78f4fea4 100644 (file)
@@ -225,7 +225,6 @@ svm_migrate_get_vram_page(struct svm_range *prange, unsigned long pfn)
        page = pfn_to_page(pfn);
        svm_range_bo_ref(prange->svm_bo);
        page->zone_device_data = prange->svm_bo;
-       get_page(page);
        lock_page(page);
 }
 
index a5cdfbe32b5e54338796c556d216c8177f45bd98..7ba66ad68a8a1e5b1b92b32177e2c5fc8c8b23e5 100644 (file)
@@ -326,7 +326,6 @@ nouveau_dmem_page_alloc_locked(struct nouveau_drm *drm)
                        return NULL;
        }
 
-       get_page(page);
        lock_page(page);
        return page;
 }
index 6c7dc1387beb0f2ac34be3e56dd5579130c8ec13..e9433bbc48010a5ebc1678b241d677abaa466f17 100644 (file)
@@ -48,7 +48,6 @@ config FS_DAX
        bool "File system based Direct Access (DAX) support"
        depends on MMU
        depends on !(ARM || MIPS || SPARC)
-       select DEV_PAGEMAP_OPS if (ZONE_DEVICE && !FS_DAX_LIMITED)
        select FS_IOMAP
        select DAX
        help
index 514ab46f597e5c3411393b044d8571121cadc872..d6a114dd5ea8b74815277f31aca0de28cb5ed20e 100644 (file)
@@ -68,9 +68,9 @@ enum memory_type {
 
 struct dev_pagemap_ops {
        /*
-        * Called once the page refcount reaches 1.  (ZONE_DEVICE pages never
-        * reach 0 refcount unless there is a refcount bug. This allows the
-        * device driver to implement its own memory management.)
+        * Called once the page refcount reaches 0.  The reference count will be
+        * reset to one by the core code after the method is called to prepare
+        * for handing out the page again.
         */
        void (*page_free)(struct page *page);
 
@@ -133,16 +133,14 @@ static inline unsigned long pgmap_vmemmap_nr(struct dev_pagemap *pgmap)
 
 static inline bool is_device_private_page(const struct page *page)
 {
-       return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) &&
-               IS_ENABLED(CONFIG_DEVICE_PRIVATE) &&
+       return IS_ENABLED(CONFIG_DEVICE_PRIVATE) &&
                is_zone_device_page(page) &&
                page->pgmap->type == MEMORY_DEVICE_PRIVATE;
 }
 
 static inline bool is_pci_p2pdma_page(const struct page *page)
 {
-       return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) &&
-               IS_ENABLED(CONFIG_PCI_P2PDMA) &&
+       return IS_ENABLED(CONFIG_PCI_P2PDMA) &&
                is_zone_device_page(page) &&
                page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA;
 }
index cb8bee88e70cce13be4ca10e900c545b95a7f78c..0201d258c646b965f7b4fd423fd64fd18bd08b66 100644 (file)
@@ -1090,7 +1090,7 @@ static inline bool is_zone_movable_page(const struct page *page)
        return page_zonenum(page) == ZONE_MOVABLE;
 }
 
-#ifdef CONFIG_DEV_PAGEMAP_OPS
+#if defined(CONFIG_ZONE_DEVICE) && defined(CONFIG_FS_DAX)
 DECLARE_STATIC_KEY_FALSE(devmap_managed_key);
 
 bool __put_devmap_managed_page(struct page *page);
@@ -1103,12 +1103,12 @@ static inline bool put_devmap_managed_page(struct page *page)
        return __put_devmap_managed_page(page);
 }
 
-#else /* CONFIG_DEV_PAGEMAP_OPS */
+#else /* CONFIG_ZONE_DEVICE && CONFIG_FS_DAX */
 static inline bool put_devmap_managed_page(struct page *page)
 {
        return false;
 }
-#endif /* CONFIG_DEV_PAGEMAP_OPS */
+#endif /* CONFIG_ZONE_DEVICE && CONFIG_FS_DAX */
 
 /* 127: arbitrary random number, small enough to assemble well */
 #define folio_ref_zero_or_close_to_overflow(folio) \
index e5fc14ba71f33e88bc845b0c94cfd5ea837cc7ab..cfe63204783918acd80ff623e7af1300b94c3be6 100644 (file)
@@ -566,7 +566,6 @@ static struct page *dmirror_devmem_alloc_page(struct dmirror_device *mdevice)
        }
 
        dpage->zone_device_data = rpage;
-       get_page(dpage);
        lock_page(dpage);
        return dpage;
 
index 3326ee3903f33078143607ba0a67d131d567a03f..a1901ae6d06293863b9ec6a83f47e22fe2e03fc5 100644 (file)
@@ -776,9 +776,6 @@ config ZONE_DEVICE
 
          If FS_DAX is enabled, then say Y.
 
-config DEV_PAGEMAP_OPS
-       bool
-
 #
 # Helpers to mirror range of the CPU page tables of a process into device page
 # tables.
@@ -790,7 +787,6 @@ config HMM_MIRROR
 config DEVICE_PRIVATE
        bool "Unaddressable device memory (GPU memory, ...)"
        depends on ZONE_DEVICE
-       select DEV_PAGEMAP_OPS
 
        help
          Allows creation of struct pages to represent unaddressable device
index 450a2c8a43f38c6d49a4ea1bb6c5a4e6a253ef19..3756dd5d2c9204e8aadb2ce5c347e21a16605970 100644 (file)
@@ -735,4 +735,6 @@ void vunmap_range_noflush(unsigned long start, unsigned long end);
 int numa_migrate_prep(struct page *page, struct vm_area_struct *vma,
                      unsigned long addr, int page_nid, int *flags);
 
+void free_zone_device_page(struct page *page);
+
 #endif /* __MM_INTERNAL_H */
index 2c5032294c9f7e493f87deaf3ffccdd13fed92de..8fef072dc1ce8b41f402afa06ea8361d3f36df5a 100644 (file)
@@ -5503,17 +5503,12 @@ static struct page *mc_handle_swap_pte(struct vm_area_struct *vma,
                return NULL;
 
        /*
-        * Handle MEMORY_DEVICE_PRIVATE which are ZONE_DEVICE page belonging to
-        * a device and because they are not accessible by CPU they are store
-        * as special swap entry in the CPU page table.
+        * Handle device private pages that are not accessible by the CPU, but
+        * stored as special swap entries in the page table.
         */
        if (is_device_private_entry(ent)) {
                page = pfn_swap_entry_to_page(ent);
-               /*
-                * MEMORY_DEVICE_PRIVATE means ZONE_DEVICE page and which have
-                * a refcount of 1 when free (unlike normal page)
-                */
-               if (!page_ref_add_unless(page, 1, 1))
+               if (!get_page_unless_zero(page))
                        return NULL;
                return page;
        }
index a0ece2344c2cabd12427b58c95d703a27a45ac04..fef5734d5e4933c557a6b35d99e1aeb97fd8759e 100644 (file)
@@ -12,6 +12,7 @@
 #include <linux/types.h>
 #include <linux/wait_bit.h>
 #include <linux/xarray.h>
+#include "internal.h"
 
 static DEFINE_XARRAY(pgmap_array);
 
@@ -37,21 +38,19 @@ unsigned long memremap_compat_align(void)
 EXPORT_SYMBOL_GPL(memremap_compat_align);
 #endif
 
-#ifdef CONFIG_DEV_PAGEMAP_OPS
+#ifdef CONFIG_FS_DAX
 DEFINE_STATIC_KEY_FALSE(devmap_managed_key);
 EXPORT_SYMBOL(devmap_managed_key);
 
 static void devmap_managed_enable_put(struct dev_pagemap *pgmap)
 {
-       if (pgmap->type == MEMORY_DEVICE_PRIVATE ||
-           pgmap->type == MEMORY_DEVICE_FS_DAX)
+       if (pgmap->type == MEMORY_DEVICE_FS_DAX)
                static_branch_dec(&devmap_managed_key);
 }
 
 static void devmap_managed_enable_get(struct dev_pagemap *pgmap)
 {
-       if (pgmap->type == MEMORY_DEVICE_PRIVATE ||
-           pgmap->type == MEMORY_DEVICE_FS_DAX)
+       if (pgmap->type == MEMORY_DEVICE_FS_DAX)
                static_branch_inc(&devmap_managed_key);
 }
 #else
@@ -61,7 +60,7 @@ static void devmap_managed_enable_get(struct dev_pagemap *pgmap)
 static void devmap_managed_enable_put(struct dev_pagemap *pgmap)
 {
 }
-#endif /* CONFIG_DEV_PAGEMAP_OPS */
+#endif /* CONFIG_FS_DAX */
 
 static void pgmap_array_delete(struct range *range)
 {
@@ -102,23 +101,12 @@ static unsigned long pfn_end(struct dev_pagemap *pgmap, int range_id)
        return (range->start + range_len(range)) >> PAGE_SHIFT;
 }
 
-static unsigned long pfn_next(struct dev_pagemap *pgmap, unsigned long pfn)
-{
-       if (pfn % (1024 << pgmap->vmemmap_shift))
-               cond_resched();
-       return pfn + pgmap_vmemmap_nr(pgmap);
-}
-
 static unsigned long pfn_len(struct dev_pagemap *pgmap, unsigned long range_id)
 {
        return (pfn_end(pgmap, range_id) -
                pfn_first(pgmap, range_id)) >> pgmap->vmemmap_shift;
 }
 
-#define for_each_device_pfn(pfn, map, i) \
-       for (pfn = pfn_first(map, i); pfn < pfn_end(map, i); \
-            pfn = pfn_next(map, pfn))
-
 static void pageunmap_range(struct dev_pagemap *pgmap, int range_id)
 {
        struct range *range = &pgmap->ranges[range_id];
@@ -147,13 +135,11 @@ static void pageunmap_range(struct dev_pagemap *pgmap, int range_id)
 
 void memunmap_pages(struct dev_pagemap *pgmap)
 {
-       unsigned long pfn;
        int i;
 
        percpu_ref_kill(&pgmap->ref);
        for (i = 0; i < pgmap->nr_range; i++)
-               for_each_device_pfn(pfn, pgmap, i)
-                       put_page(pfn_to_page(pfn));
+               percpu_ref_put_many(&pgmap->ref, pfn_len(pgmap, i));
        wait_for_completion(&pgmap->done);
        percpu_ref_exit(&pgmap->ref);
 
@@ -464,14 +450,10 @@ struct dev_pagemap *get_dev_pagemap(unsigned long pfn,
 }
 EXPORT_SYMBOL_GPL(get_dev_pagemap);
 
-#ifdef CONFIG_DEV_PAGEMAP_OPS
-void free_devmap_managed_page(struct page *page)
+void free_zone_device_page(struct page *page)
 {
-       /* notify page idle for dax */
-       if (!is_device_private_page(page)) {
-               wake_up_var(&page->_refcount);
+       if (WARN_ON_ONCE(!is_device_private_page(page)))
                return;
-       }
 
        __ClearPageWaiters(page);
 
@@ -500,28 +482,27 @@ void free_devmap_managed_page(struct page *page)
         */
        page->mapping = NULL;
        page->pgmap->ops->page_free(page);
+
+       /*
+        * Reset the page count to 1 to prepare for handing out the page again.
+        */
+       set_page_count(page, 1);
 }
 
+#ifdef CONFIG_FS_DAX
 bool __put_devmap_managed_page(struct page *page)
 {
-       if (page->pgmap->type != MEMORY_DEVICE_PRIVATE &&
-           page->pgmap->type != MEMORY_DEVICE_FS_DAX)
+       if (page->pgmap->type != MEMORY_DEVICE_FS_DAX)
                return false;
 
        /*
-        * devmap page refcounts are 1-based, rather than 0-based: if
+        * fsdax page refcounts are 1-based, rather than 0-based: if
         * refcount is 1, then the page is free and the refcount is
         * stable because nobody holds a reference on the page.
         */
-       switch (page_ref_dec_return(page)) {
-       case 1:
-               free_devmap_managed_page(page);
-               break;
-       case 0:
-               __put_page(page);
-               break;
-       }
+       if (page_ref_dec_return(page) == 1)
+               wake_up_var(&page->_refcount);
        return true;
 }
 EXPORT_SYMBOL(__put_devmap_managed_page);
-#endif /* CONFIG_DEV_PAGEMAP_OPS */
+#endif /* CONFIG_FS_DAX */
index e7d0b68d5dcb3dedbdb36d9258d84ee0d4dceeeb..af0534de618ab25e9d74219b3d674c86bb5e7763 100644 (file)
@@ -338,14 +338,8 @@ static int expected_page_refs(struct address_space *mapping, struct page *page)
 {
        int expected_count = 1;
 
-       /*
-        * Device private pages have an extra refcount as they are
-        * ZONE_DEVICE pages.
-        */
-       expected_count += is_device_private_page(page);
        if (mapping)
                expected_count += compound_nr(page) + page_has_private(page);
-
        return expected_count;
 }
 
index db8d0eea13d72c677176e299018ad388e0df1f7a..fc3b7989f5b20fa4e459d840ab3ec219b46e8e7e 100644 (file)
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -122,17 +122,9 @@ static void __put_compound_page(struct page *page)
 
 void __put_page(struct page *page)
 {
-       if (is_zone_device_page(page)) {
-               put_dev_pagemap(page->pgmap);
-
-               /*
-                * The page belongs to the device that created pgmap. Do
-                * not return it to page allocator.
-                */
-               return;
-       }
-
-       if (unlikely(PageCompound(page)))
+       if (unlikely(is_zone_device_page(page)))
+               free_zone_device_page(page);
+       else if (unlikely(PageCompound(page)))
                __put_compound_page(page);
        else
                __put_single_page(page);
@@ -933,7 +925,7 @@ void release_pages(struct page **pages, int nr)
                        if (put_devmap_managed_page(page))
                                continue;
                        if (put_page_testzero(page))
-                               put_dev_pagemap(page->pgmap);
+                               free_zone_device_page(page);
                        continue;
                }