Re: [PATCH 2/6] mm/page_alloc: Use only one PCP list for THP-sized allocations

[Vlastimil Babka <vbabka@suse.cz>]

On 5/12/22 10:50, Mel Gorman wrote:
> The per_cpu_pages is cache-aligned on a standard x86-64 distribution
> configuration but a later patch will add a new field which would push
> the structure into the next cache line. Use only one list to store
> THP-sized pages on the per-cpu list. This assumes that the vast majority
> of THP-sized allocations are GFP_MOVABLE but even if it was another type,
> it would not contribute to serious fragmentation that potentially causes
> a later THP allocation failure. Align per_cpu_pages on the cacheline
> boundary to ensure there is no false cache sharing.
> 
> After this patch, the structure sizing is;
> 
> struct per_cpu_pages {
>         int                        count;                /*     0     4 */
>         int                        high;                 /*     4     4 */
>         int                        batch;                /*     8     4 */
>         short int                  free_factor;          /*    12     2 */
>         short int                  expire;               /*    14     2 */
>         struct list_head           lists[13];            /*    16   208 */
> 
>         /* size: 256, cachelines: 4, members: 6 */
>         /* padding: 32 */
> } __attribute__((__aligned__(64)));
> 
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> Tested-by: Minchan Kim <minchan@kernel.org>
> Acked-by: Minchan Kim <minchan@kernel.org>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  include/linux/mmzone.h | 11 +++++++----
>  mm/page_alloc.c        |  4 ++--
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 962b14d403e8..abe530748de6 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -358,15 +358,18 @@ enum zone_watermarks {
>  };
>  
>  /*
> - * One per migratetype for each PAGE_ALLOC_COSTLY_ORDER plus one additional
> - * for pageblock size for THP if configured.
> + * One per migratetype for each PAGE_ALLOC_COSTLY_ORDER. One additional list
> + * for THP which will usually be GFP_MOVABLE. Even if it is another type,
> + * it should not contribute to serious fragmentation causing THP allocation
> + * failures.
>   */
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  #define NR_PCP_THP 1
>  #else
>  #define NR_PCP_THP 0
>  #endif
> -#define NR_PCP_LISTS (MIGRATE_PCPTYPES * (PAGE_ALLOC_COSTLY_ORDER + 1 + NR_PCP_THP))
> +#define NR_LOWORDER_PCP_LISTS (MIGRATE_PCPTYPES * (PAGE_ALLOC_COSTLY_ORDER + 1))
> +#define NR_PCP_LISTS (NR_LOWORDER_PCP_LISTS + NR_PCP_THP)
>  
>  /*
>   * Shift to encode migratetype and order in the same integer, with order
> @@ -392,7 +395,7 @@ struct per_cpu_pages {
>  
>  	/* Lists of pages, one per migrate type stored on the pcp-lists */
>  	struct list_head lists[NR_PCP_LISTS];
> -};
> +} ____cacheline_aligned_in_smp;
>  
>  struct per_cpu_zonestat {
>  #ifdef CONFIG_SMP
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f58f85fdb05f..5851ee88a89c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -648,7 +648,7 @@ static inline unsigned int order_to_pindex(int migratetype, int order)
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  	if (order > PAGE_ALLOC_COSTLY_ORDER) {
>  		VM_BUG_ON(order != pageblock_order);
> -		base = PAGE_ALLOC_COSTLY_ORDER + 1;
> +		return NR_LOWORDER_PCP_LISTS;
>  	}
>  #else
>  	VM_BUG_ON(order > PAGE_ALLOC_COSTLY_ORDER);
> @@ -662,7 +662,7 @@ static inline int pindex_to_order(unsigned int pindex)
>  	int order = pindex / MIGRATE_PCPTYPES;
>  
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -	if (order > PAGE_ALLOC_COSTLY_ORDER)
> +	if (pindex == NR_LOWORDER_PCP_LISTS)
>  		order = pageblock_order;
>  #else
>  	VM_BUG_ON(order > PAGE_ALLOC_COSTLY_ORDER);