linux-kernel - Re: [PATCH RFC 06/37] mm: page_alloc: Allocate from movable pcp lists only if ALLOC_FROM

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: <20231010074823.GA2536665@tiffany>
Date:   Thu, 12 Oct 2023 10:25:11 +0900
From:   Hyesoo Yu <hyesoo.yu@...sung.com>
To:     Alexandru Elisei <alexandru.elisei@....com>
Cc:     catalin.marinas@....com, will@...nel.org, oliver.upton@...ux.dev,
        maz@...nel.org, james.morse@....com, suzuki.poulose@....com,
        yuzenghui@...wei.com, arnd@...db.de, akpm@...ux-foundation.org,
        mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com, vschneid@...hat.com, mhiramat@...nel.org,
        rppt@...nel.org, hughd@...gle.com, pcc@...gle.com,
        steven.price@....com, anshuman.khandual@....com,
        vincenzo.frascino@....com, david@...hat.com, eugenis@...gle.com,
        kcc@...gle.com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, kvmarm@...ts.linux.dev,
        linux-fsdevel@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-mm@...ck.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH RFC 06/37] mm: page_alloc: Allocate from movable pcp
 lists only if ALLOC_FROM_METADATA

On Wed, Aug 23, 2023 at 02:13:19PM +0100, Alexandru Elisei wrote:
> pcp lists keep MIGRATE_METADATA pages on the MIGRATE_MOVABLE list. Make
> sure pages from the movable list are allocated only when the
> ALLOC_FROM_METADATA alloc flag is set, as otherwise the page allocator
> could end up allocating a metadata page when that page cannot be used.
> 
> __alloc_pages_bulk() sidesteps rmqueue() and calls __rmqueue_pcplist()
> directly. Add a check for the flag before calling __rmqueue_pcplist(), and
> fallback to __alloc_pages() if the check is false.
> 
> Note that CMA isn't a problem for __alloc_pages_bulk(): an allocation can
> always use CMA pages if the requested migratetype is MIGRATE_MOVABLE, which
> is not the case with MIGRATE_METADATA pages.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@....com>
> ---
>  mm/page_alloc.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 829134a4dfa8..a693e23c4733 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2845,11 +2845,16 @@ struct page *rmqueue(struct zone *preferred_zone,
>  
>  	if (likely(pcp_allowed_order(order))) {
>  		/*
> -		 * MIGRATE_MOVABLE pcplist could have the pages on CMA area and
> -		 * we need to skip it when CMA area isn't allowed.
> +		 * PCP lists keep MIGRATE_CMA/MIGRATE_METADATA pages on the same
> +		 * movable list. Make sure it's allowed to allocate both type of
> +		 * pages before allocating from the movable list.
>  		 */
> -		if (!IS_ENABLED(CONFIG_CMA) || alloc_flags & ALLOC_CMA ||
> -				migratetype != MIGRATE_MOVABLE) {
> +		bool movable_allowed = (!IS_ENABLED(CONFIG_CMA) ||
> +					(alloc_flags & ALLOC_CMA)) &&
> +				       (!IS_ENABLED(CONFIG_MEMORY_METADATA) ||
> +					(alloc_flags & ALLOC_FROM_METADATA));
> +
> +		if (migratetype != MIGRATE_MOVABLE || movable_allowed) {

Hi!

I don't think it would be effcient when the majority of movable pages
do not use GFP_TAGGED.

Metadata pages have a low probability of being in the pcp list
because metadata pages is bypassed when freeing pages.

The allocation performance of most movable pages is likely to decrease
if only the request with ALLOC_FROM_METADATA could be allocated.

How about not including metadata pages in the pcp list at all ?

Thanks,
Hyesoo Yu.

>  			page = rmqueue_pcplist(preferred_zone, zone, order,
>  					migratetype, alloc_flags);
>  			if (likely(page))
> @@ -4388,6 +4393,14 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
>  		goto out;
>  	gfp = alloc_gfp;
>  
> +	/*
> +	 * pcp lists puts MIGRATE_METADATA on the MIGRATE_MOVABLE list, don't
> +	 * use pcp if allocating metadata pages is not allowed.
> +	 */
> +	if (metadata_storage_enabled() && ac.migratetype == MIGRATE_MOVABLE &&
> +	    !(alloc_flags & ALLOC_FROM_METADATA))
> +		goto failed;
> +
>  	/* Find an allowed local zone that meets the low watermark. */
>  	for_each_zone_zonelist_nodemask(zone, z, ac.zonelist, ac.highest_zoneidx, ac.nodemask) {
>  		unsigned long mark;
> -- 
> 2.41.0
> 
>