[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231011124610.4punxroovolyvmgr@techsingularity.net>
Date: Wed, 11 Oct 2023 13:46:10 +0100
From: Mel Gorman <mgorman@...hsingularity.net>
To: Huang Ying <ying.huang@...el.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
Arjan Van De Ven <arjan@...ux.intel.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Vlastimil Babka <vbabka@...e.cz>,
David Hildenbrand <david@...hat.com>,
Johannes Weiner <jweiner@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Michal Hocko <mhocko@...e.com>,
Pavel Tatashin <pasha.tatashin@...een.com>,
Matthew Wilcox <willy@...radead.org>,
Christoph Lameter <cl@...ux.com>
Subject: Re: [PATCH 01/10] mm, pcp: avoid to drain PCP when process exit
On Wed, Sep 20, 2023 at 02:18:47PM +0800, Huang Ying wrote:
> In commit f26b3fa04611 ("mm/page_alloc: limit number of high-order
> pages on PCP during bulk free"), the PCP (Per-CPU Pageset) will be
> drained when PCP is mostly used for high-order pages freeing to
> improve the cache-hot pages reusing between page allocation and
> freeing CPUs.
>
> But, the PCP draining mechanism may be triggered unexpectedly when
> process exits. With some customized trace point, it was found that
> PCP draining (free_high == true) was triggered with the order-1 page
> freeing with the following call stack,
>
> => free_unref_page_commit
> => free_unref_page
> => __mmdrop
> => exit_mm
> => do_exit
> => do_group_exit
> => __x64_sys_exit_group
> => do_syscall_64
>
> Checking the source code, this is the page table PGD
> freeing (mm_free_pgd()). It's a order-1 page freeing if
> CONFIG_PAGE_TABLE_ISOLATION=y. Which is a common configuration for
> security.
>
> Just before that, page freeing with the following call stack was
> found,
>
> => free_unref_page_commit
> => free_unref_page_list
> => release_pages
> => tlb_batch_pages_flush
> => tlb_finish_mmu
> => exit_mmap
> => __mmput
> => exit_mm
> => do_exit
> => do_group_exit
> => __x64_sys_exit_group
> => do_syscall_64
>
> So, when a process exits,
>
> - a large number of user pages of the process will be freed without
> page allocation, it's highly possible that pcp->free_factor becomes
> > 0.
>
> - after freeing all user pages, the PGD will be freed, which is a
> order-1 page freeing, PCP will be drained.
>
> All in all, when a process exits, it's high possible that the PCP will
> be drained. This is an unexpected behavior.
>
> To avoid this, in the patch, the PCP draining will only be triggered
> for 2 consecutive high-order page freeing.
>
> On a 2-socket Intel server with 224 logical CPU, we tested kbuild on
> one socket with `make -j 112`. With the patch, the build time
> decreases 3.4% (from 206s to 199s). The cycles% of the spinlock
> contention (mostly for zone lock) decreases from 43.6% to 40.3% (with
> PCP size == 361). The number of PCP draining for high order pages
> freeing (free_high) decreases 50.8%.
>
> This helps network workload too for reduced zone lock contention. On
> a 2-socket Intel server with 128 logical CPU, with the patch, the
> network bandwidth of the UNIX (AF_UNIX) test case of lmbench test
> suite with 16-pair processes increase 17.1%. The cycles% of the
> spinlock contention (mostly for zone lock) decreases from 50.0% to
> 45.8%. The number of PCP draining for high order pages
> freeing (free_high) decreases 27.4%. The cache miss rate keeps 0.3%.
>
> Signed-off-by: "Huang, Ying" <ying.huang@...el.com>
Acked-by: Mel Gorman <mgorman@...hsingularity.net>
However, I want to note that batching on exit is not necessarily
unexpected. For processes that are multi-TB in size, the time to exit
can actually be quite large and batching is of benefit but optimising
for exit is rarely a winning strategy. The pattern of "all allocs on CPU
B and all frees on CPU B" or "short-lived tasks triggering a premature
drain" is a bit more compelling but not worth a changelog rewrite.
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 4106fbc5b4b3..64d5ed2bb724 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -676,12 +676,15 @@ enum zone_watermarks {
> #define high_wmark_pages(z) (z->_watermark[WMARK_HIGH] + z->watermark_boost)
> #define wmark_pages(z, i) (z->_watermark[i] + z->watermark_boost)
>
> +#define PCPF_PREV_FREE_HIGH_ORDER 0x01
> +
The meaning of the flag and its intent should have been documented.
--
Mel Gorman
SUSE Labs
Powered by blists - more mailing lists