On 7/15/21 1:52 PM, Joao Martins wrote:
> On 7/15/21 2:08 AM, Dan Williams wrote:
>> On Wed, Jul 14, 2021 at 12:36 PM Joao Martins <joao.m.martins@xxxxxxxxxx> wrote:
>>> 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 <joao.m.martins@xxxxxxxxxx>
>>> ---
>>> 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.
