On Wed, Jul 28, 2021 at 8:36 AM Joao Martins <[email protected]> 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?
[..]
> [...] And here's compound_section_tail_huge_page() (for the last patch in the
> series):
>
>
> @@ -690,6 +727,33 @@ static struct page * __meminit
> compound_section_tail_page(unsigned
> long addr)
> return pte_page(*ptep);
> }
>
> +static struct page * __meminit compound_section_tail_huge_page(unsigned long
> addr,
> + unsigned long offset, struct dev_pagemap
> *pgmap)
> +{
> + unsigned long geometry_size = pgmap_geometry(pgmap) << PAGE_SHIFT;
> + pmd_t *pmdp;
> +
> + addr -= PAGE_SIZE;
> +
> + /*
> + * Assuming sections are populated sequentially, the previous
> section's
> + * page data can be reused.
> + */
> + pmdp = pmd_off_k(addr);
> + if (!pmdp)
> + return ERR_PTR(-ENOMEM);
> +
> + /*
> + * Reuse the tail pages vmemmap pmd page
> + * See layout diagram in Documentation/vm/vmemmap_dedup.rst
> + */
> + if (offset % geometry_size > PFN_PHYS(PAGES_PER_SECTION))
> + return pmd_page(*pmdp);
> +
> + /* No reusable PMD fallback to PTE tail page*/
> + return NULL;
> +}
> +
> static int __meminit vmemmap_populate_compound_pages(unsigned long start_pfn,
> unsigned long start,
> unsigned long end, int
> node,
> @@ -697,14 +761,22 @@ static int __meminit
> vmemmap_populate_compound_pages(unsigned long
> start_pfn,
> {
> unsigned long offset, size, addr;
>
> - if (compound_section_index(start_pfn, pgmap)) {
> - struct page *page;
> + if (compound_section_index(start_pfn, pgmap, &offset)) {
> + struct page *page, *hpage;
> +
> + hpage = compound_section_tail_huge_page(addr, offset);
> + if (IS_ERR(hpage))
> + return -ENOMEM;
> + else if (hpage)
No need for "else" after return... other than that these helpers and
this arrangement looks good to me.
