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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <80c89ad9-17ec-8d64-3774-821451e147c4@suse.cz>
Date:   Thu, 19 May 2022 11:45:35 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Mel Gorman <mgorman@...hsingularity.net>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     Nicolas Saenz Julienne <nsaenzju@...hat.com>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        Michal Hocko <mhocko@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>
Subject: Re: [PATCH 2/6] mm/page_alloc: Use only one PCP list for THP-sized
 allocations

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@...hsingularity.net>
> Tested-by: Minchan Kim <minchan@...nel.org>
> Acked-by: Minchan Kim <minchan@...nel.org>

Acked-by: Vlastimil Babka <vbabka@...e.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);