Re: [PATCH v3 08/14] mm/sparse-vmemmap: populate compound pagemaps

[Joao Martins <joao.m.martins@oracle.com>]

On 7/28/21 7:54 PM, Joao Martins wrote:
> 
> 
> On 7/28/21 7:03 PM, Dan Williams wrote:
>> On Wed, Jul 28, 2021 at 8:36 AM Joao Martins <joao.m.martins@oracle.com> wrote:
>> [..]
>>> +/*
>>> + * For compound pages bigger than section size (e.g. x86 1G compound
>>> + * pages with 2M subsection size) fill the rest of sections as tail
>>> + * pages.
>>> + *
>>> + * Note that memremap_pages() resets @nr_range value and will increment
>>> + * it after each range successful onlining. Thus the value or @nr_range
>>> + * at section memmap populate corresponds to the in-progress range
>>> + * being onlined here.
>>> + */
>>> +static bool compound_section_index(unsigned long start_pfn,
>>
>> Oh, I was thinking this would return the actual Nth index number for
>> the section within the compound page. 
>> A bool is ok too, but then the
>> function name would be something like:
>>
>> reuse_compound_section()
>>
>> ...right?
>>
> Yes.
> 
Additionally, I am shifting calculations to be PFN based to avoid needless conversions of
@geometry to bytes. So from this:

+static bool __meminit compound_section_index(unsigned long start_pfn,
+                                            struct dev_pagemap *pgmap)
+{
+       unsigned long geometry_size = pgmap_geometry(pgmap) << PAGE_SHIFT;
+       unsigned long offset = PFN_PHYS(start_pfn) -
+               pgmap->ranges[pgmap->nr_range].start;
+
+       return !IS_ALIGNED(offset, geometry_size) &&
+               geometry_size > SUBSECTION_SIZE;
+}

To this:

+static bool __meminit reuse_compound_section(unsigned long start_pfn,
+                                            struct dev_pagemap *pgmap)
+{
+       unsigned long geometry = pgmap_geometry(pgmap);
+       unsigned long offset = start_pfn -
+               PHYS_PFN(pgmap->ranges[pgmap->nr_range].start);
+
+       return !IS_ALIGNED(offset, geometry) && geometry > PAGES_PER_SUBSECTION;
+}