[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87zg0odob1.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Thu, 12 Oct 2023 20:21:06 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Mel Gorman <mgorman@...hsingularity.net>
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
Mel Gorman <mgorman@...hsingularity.net> writes:
> 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.
Sure. Will add comments for the flags.
--
Best Regards,
Huang, Ying
Powered by blists - more mailing lists