Re: [PATCH 06/17] mm/sparse-vmemmap: support section-based vmemmap accounting

[Mike Rapoport]

> section_nr_vmemmap_pages() is used to account the vmemmap pages consumed by a
> memory section, but it currently only understands the ordinary case and the
> pgmap-provided optimization case. That is not enough for section-based vmemmap
> optimization, where the compound page order is carried by the memory section
> itself and tail vmemmap pages may be shared.
> 
> Make the helper report the actual vmemmap footprint of a section, so it can be
> used as the common accounting path for both ordinary and optimized sections.
> 
> Teach section_nr_vmemmap_pages() to use the section order when there is no
> pgmap, and to account only the vmemmap pages that are actually needed for an
> optimizable section. This keeps the accounting consistent with section-based
> vmemmap optimization.
> 
> Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 6fa6e7f0abf9..41a1ebbe5e85 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -2403,6 +2403,14 @@ static inline unsigned int section_order(const struct mem_section *section)
>  }
>  #endif
>  
> +static inline bool section_vmemmap_optimizable(const struct mem_section *section)
> +{
> +	if (!is_power_of_2(sizeof(struct page)))
> +		return false;
> +
> +	return section_order(section) >= OPTIMIZABLE_FOLIO_MIN_ORDER;

Like with page_vmemmap_optimizable() I think it's not obvious that
section_order() is enough to gate vmemmap optimization.

> +}
> +
>  /*
>   * Fallback case for when the architecture provides its own pfn_valid() but
>   * not a corresponding for_each_valid_pfn().
> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> index 77154e9da08c..99f59394feab 100644
> --- a/mm/sparse-vmemmap.c
> +++ b/mm/sparse-vmemmap.c
> @@ -645,24 +645,29 @@ void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
>  static int __meminit section_nr_vmemmap_pages(unsigned long pfn, unsigned long nr_pages,
>  		struct vmem_altmap *altmap, struct dev_pagemap *pgmap)
>  {
> -	const unsigned int order = pgmap ? pgmap->vmemmap_shift : 0;
> +	const struct mem_section *ms = __pfn_to_section(pfn);
> +	const unsigned int order = pgmap ? pgmap->vmemmap_shift : section_order(ms);
>  	const unsigned long pages_per_compound = 1UL << order;
> +	unsigned int vmemmap_pages = OPTIMIZED_FOLIO_VMEMMAP_PAGES;
>  
>  	VM_WARN_ON_ONCE(!IS_ALIGNED(pfn | nr_pages, PAGES_PER_SUBSECTION));
>  	VM_WARN_ON_ONCE(nr_pages > PAGES_PER_SECTION);
>  
> -	if (!vmemmap_can_optimize(altmap, pgmap))
> +	if (vmemmap_can_optimize(altmap, pgmap))
> +		vmemmap_pages = VMEMMAP_RESERVE_NR;

I'd put it below the next if ...

> +
> +	if (!vmemmap_can_optimize(altmap, pgmap) && !section_vmemmap_optimizable(ms))
>  		return DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SIZE);

... and make it

	vmmemap_pages = vmemmap_can_optimize() ? VMEMMAP_RESERVE_NR : 1;

-- 
Sincerely yours,
Mike.