linux-kernel - Re: [PATCH V2] mm: compaction: skip memory compaction when there are not enough migratable pages

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: <324bf85f-442a-4388-a8f4-55d60e57b914@linux.alibaba.com>
Date: Mon, 13 Jan 2025 16:11:35 +0800
From: Baolin Wang <baolin.wang@...ux.alibaba.com>
To: yangge1116@....com, akpm@...ux-foundation.org
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org, 21cnbao@...il.com,
 david@...hat.com, hannes@...xchg.org, liuzixing@...on.cn,
 Vlastimil Babka <vbabka@...e.cz>
Subject: Re: [PATCH V2] mm: compaction: skip memory compaction when there are
 not enough migratable pages

Cc Vlastimil.

On 2025/1/8 16:37, yangge1116@....com wrote:
> From: yangge <yangge1116@....com>
> 
> There are 4 NUMA nodes on my machine, and each NUMA node has 32GB
> of memory. I have configured 16GB of CMA memory on each NUMA node,
> and starting a 32GB virtual machine with device passthrough is
> extremely slow, taking almost an hour.
> 
> During the start-up of the virtual machine, it will call
> pin_user_pages_remote(..., FOLL_LONGTERM, ...) to allocate memory.
> Long term GUP cannot allocate memory from CMA area, so a maximum of
> 16 GB of no-CMA memory on a NUMA node can be used as virtual machine
> memory. There is 16GB of free CMA memory on a NUMA node, which is
> sufficient to pass the order-0 watermark check, causing the
> __compaction_suitable() function to  consistently return true.
> However, if there aren't enough migratable pages available, performing
> memory compaction is also meaningless. Besides checking whether
> the order-0 watermark is met, __compaction_suitable() also needs
> to determine whether there are sufficient migratable pages available
> for memory compaction.
> 
> For costly allocations, because __compaction_suitable() always
> returns true, __alloc_pages_slowpath() can't exit at the appropriate
> place, resulting in excessively long virtual machine startup times.
> Call trace:
> __alloc_pages_slowpath
>      if (compact_result == COMPACT_SKIPPED ||
>          compact_result == COMPACT_DEFERRED)
>          goto nopage; // should exit __alloc_pages_slowpath() from here
> 
> When the 16G of non-CMA memory on a single node is exhausted, we will
> fallback to allocating memory on other nodes. In order to quickly
> fallback to remote nodes, we should skip memory compaction when
> migratable pages are insufficient. After this fix, it only takes a
> few tens of seconds to start a 32GB virtual machine with device
> passthrough functionality.
> 
> Signed-off-by: yangge <yangge1116@....com>
> ---
> 
> V2:
> - consider unevictable folios
> 
>   mm/compaction.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 07bd227..1630abd 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2383,7 +2383,27 @@ static bool __compaction_suitable(struct zone *zone, int order,
>   				  int highest_zoneidx,
>   				  unsigned long wmark_target)
>   {
> +	struct pglist_data *pgdat = zone->zone_pgdat;
> +	unsigned long sum, nr_pinned;
>   	unsigned long watermark;
> +
> +	sum = node_page_state(pgdat, NR_INACTIVE_FILE) +
> +		node_page_state(pgdat, NR_INACTIVE_ANON) +
> +		node_page_state(pgdat, NR_ACTIVE_FILE) +
> +		node_page_state(pgdat, NR_ACTIVE_ANON) +
> +		node_page_state(pgdat, NR_UNEVICTABLE);
> +
> +	nr_pinned = node_page_state(pgdat, NR_FOLL_PIN_ACQUIRED) -
> +		node_page_state(pgdat, NR_FOLL_PIN_RELEASED);
> +
> +	/*
> +	 * Gup-pinned pages are non-migratable. After subtracting these pages,
> +	 * we need to check if the remaining pages are sufficient for memory
> +	 * compaction.
> +	 */
> +	if ((sum - nr_pinned) < (1 << order))
> +		return false;
> +

Looks reasonable to me, but let's see if other people have any comments.