Re: [PATCH v3 04/14] mm/memremap: add ZONE_DEVICE support for compound pages

On 7/15/21 1:52 PM, Joao Martins wrote:

On 7/15/21 2:08 AM, Dan Williams wrote:

quoted

On Wed, Jul 14, 2021 at 12:36 PM Joao Martins [off-list ref] wrote:

quoted

instead of
base pages. When a compound page geometry is requested, all but the first
page are initialised as tail pages instead of order-0 pages.

For certain ZONE_DEVICE users like device-dax which have a fixed page size,
this creates an opportunity to optimize GUP and GUP-fast walkers, treating
it the same way as THP or hugetlb pages.

Signed-off-by: Joao Martins <redacted>
---
 include/linux/memremap.h | 17 +++++++++++++++++
 mm/memremap.c            |  8 ++++++--
 mm/page_alloc.c          | 34 +++++++++++++++++++++++++++++++++-
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 119f130ef8f1..e5ab6d4525c1 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h

@@ -99,6 +99,10 @@ struct dev_pagemap_ops {
  * @done: completion for @internal_ref
  * @type: memory type: see MEMORY_* in memory_hotplug.h
  * @flags: PGMAP_* flags to specify defailed behavior
+ * @geometry: structural definition of how the vmemmap metadata is populated.
+ *     A zero or PAGE_SIZE defaults to using base pages as the memmap metadata
+ *     representation. A bigger value but also multiple of PAGE_SIZE will set
+ *     up compound struct pages representative of the requested geometry size.
  * @ops: method table
  * @owner: an opaque pointer identifying the entity that manages this
  *     instance.  Used by various helpers to make sure that no

@@ -114,6 +118,7 @@ struct dev_pagemap {
        struct completion done;
        enum memory_type type;
        unsigned int flags;
+       unsigned long geometry;
        const struct dev_pagemap_ops *ops;
        void *owner;
        int nr_range;

@@ -130,6 +135,18 @@ static inline struct vmem_altmap *pgmap_altmap(struct dev_pagemap *pgmap)
        return NULL;
 }

+static inline unsigned long pgmap_geometry(struct dev_pagemap *pgmap)
+{
+       if (!pgmap || !pgmap->geometry)
+               return PAGE_SIZE;
+       return pgmap->geometry;
+}
+
+static inline unsigned long pgmap_pfn_geometry(struct dev_pagemap *pgmap)
+{
+       return PHYS_PFN(pgmap_geometry(pgmap));
+}

Are both needed? Maybe just have ->geometry natively be in nr_pages
units directly, because pgmap_pfn_geometry() makes it confusing
whether it's a geometry of the pfn or the geometry of the pgmap.

I use pgmap_geometry() largelly when we manipulate memmap in sparse-vmemmap code, as we
deal with addresses/offsets/subsection-size. While using pgmap_pfn_geometry for code that
deals with PFN initialization. For this patch I could remove the confusion.

And actually maybe I can just store the pgmap_geometry() value in bytes locally in
vmemmap_populate_compound_pages() and we can remove this extra helper.

But one nice property of pgmap_geometry() is the @pgmap check and not needing that the
driver initializes a pgmap->geometry property. So a zeroed value would still support pgmap
users on the old case where there's no @geometry (or the user doesn't care). So departing
from this helper might mean that either memremap_pages() sets the right @geometry if a
zeroed value is passed in. and __populate_section_memmap() makes sures pgmap is associated
when trying to figure out if there's a geometry to consider in the section mapping.