From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D3023C6FD1F for ; Wed, 3 Apr 2024 08:47:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6973C6B0095; Wed, 3 Apr 2024 04:47:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 647C16B0098; Wed, 3 Apr 2024 04:47:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 50FD26B0099; Wed, 3 Apr 2024 04:47:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 28FB16B0095 for ; Wed, 3 Apr 2024 04:47:15 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id C1C8C40DF4 for ; Wed, 3 Apr 2024 08:47:14 +0000 (UTC) X-FDA: 81967591188.26.7650E89 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf01.hostedemail.com (Postfix) with ESMTP id 5D0734000D for ; Wed, 3 Apr 2024 08:47:04 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf01.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712134024; a=rsa-sha256; cv=none; b=OedmfxETlUg1sqgCN9FSVUUjd3Va0OV/yoaehVeqiLceccrtnB127QLnYXmcQSO0yK6Ip6 sdtpfTGC+86yVPR5BtR1IihSQGqx/JgyGyK7M6i9esydkawoFWBkQpy7KRn96XaeIOOvuS FBPRLiZivAINdza+/qVYXNC4tICpyV4= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf01.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712134024; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=eybQKzgS9gUeyPaUqVNZA963yibkYjlaSa6tLTlS/Qc=; b=OKuTomLjiCJm96hSAkWT3Sb6waeCCYW7iAzgzqiAGNEE8yFjgfsGsXqEzWE04epYKrFZLO L1COT6yKvZUiqBUJaaATVIiIPYjJTvWWkCyvM8f9ObKq3mHYW6ypbj2RJi3SeWXMjZu2iH EuBIIDloNqgQ0QhwdbbOXfzSUjafiWU= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id ABE6D1007; Wed, 3 Apr 2024 01:47:34 -0700 (PDT) Received: from [10.57.72.245] (unknown [10.57.72.245]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 226E93F7B4; Wed, 3 Apr 2024 01:47:00 -0700 (PDT) Message-ID: <502e5ff9-9b3c-441d-8bb5-bf94b78a4aa4@arm.com> Date: Wed, 3 Apr 2024 09:46:59 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] mm: add per-order mTHP alloc_success and alloc_fail counters Content-Language: en-GB To: David Hildenbrand , Barry Song <21cnbao@gmail.com> Cc: akpm@linux-foundation.org, linux-mm@kvack.org, willy@infradead.org, cerasuolodomenico@gmail.com, kasong@tencent.com, surenb@google.com, v-songbaohua@oppo.com, yosryahmed@google.com, yuzhao@google.com, chrisl@kernel.org, peterx@redhat.com References: <20240328095139.143374-1-21cnbao@gmail.com> <56027891-090b-4501-ae0c-a86077caf303@redhat.com> From: Ryan Roberts In-Reply-To: <56027891-090b-4501-ae0c-a86077caf303@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 5D0734000D X-Stat-Signature: 8chs6zjtmmn8wqo5bkktudjhns1mjj6r X-Rspam-User: X-HE-Tag: 1712134024-438828 X-HE-Meta: U2FsdGVkX1/KvZojODgRIQToOeA3/jKh6RreGK5+cgOs4bx5t+I17wyS2OptERd8U63/v1jqTQM4YF5nH4BiTGBhlN6ZDzXI1OnD6gJUSgCPhxeLlAOGrnxrJFGOhiv+KhJuFtptcih8nA5hBhl4YQJ/Na/iRv/XYCG+LYXZud1TKfumHF5RQNuVgnFbzAog+VPjHslDuLgXW+4Q2wts39teoEY551kcoJr8UUFteBpjkv7WcWC8VxzOWNlU/8m8CIebiDBP+HMBajq7rHkbj1p1SFvJEAG5rD63eH/KAiJIv/NLJiId7sxotZB1g9IzbXEhb8F/2MJUIVA5lvAbY2LDC3fPcFgBdFzwwTUQ1C0WlWnfz4/6zUlV7ZTrBPTn2r/KxOrfNnbR+SB8qpYfDX2GfOdumXaR5XuAyLl1LeZdvJ6csTRWAKJbwAA2HKxrGtpQ9yc5dNC/SW5mFhfkxkOYkT0r1V5e3iT3Op6AXBA0apuy4yd7vMlN2c11KRgWMXUGWF9Yv9lvLJNdSlNT2f+4spjh8KiapJk4ONpqQWUNtBqx0rIakPUpByR7QublUiQVTNNr8vY6MKVsMiO1SdkPNkTJpEbuFbJattT2AjASGfW3pPGNfhln3CxvLKQ1tBC4U7Wp7Z3ggFfwmEeWix/HGo/664K5loSqp3FZUsn4TrRMYnGjiLb4T8S/2TIz0bP4sXsg42bkG99ANgRxCesnmYvQaomrjToYEO7TRVmkA9zqskn6ubu/35Tr0BG70Mk5xmyuixEQ0yM5XF/bhlBk2T8oa5+0AR+b/dYNJyGLrJ491uKqJww1SovY5tdNQa3a3viSvibVCtji6sPGCZVFG1aOlWKqMgTkKqBfRFQe7ulv/ZfSkyE6GMZ7U0mfPUR4etteSUgiHwOC9tO0xqqJaPwLOHHQ4sC+gG1SM6fcV9Afj9pirM21AfNBxaUUyghC7J77eFlsZRB64qu N4Y4se6h u1L+YYfyockG0+ot4sz40DFCRnHBFqLgGwSsKMj5NJbiYtu39NAD5Jx2JifpJio1DI7bX65JTRK01v7KABk0hYXx4rPjgsH93M/O6ZQYJt8zshY1LoWQUH3G55HZNw/R131H0xdYZmlGEuHlPe1T+XdHPUt7/xZ9dmUxSF59pPjU7k1251Czvb13B18b8NJnmFPXBcSfA6+bnyvmkeTR/Pj8X5hzZgVZ6wqRUZhwC/HpGQfPkeIhRDA75jk71kWB7obOOqMITERrnXnEo15tuotjQnj+sushhIGH0d2nMMCCQ0RFEkmY/KOxcNeOzMyt4SlAINE742DFupUItC+5bkALPjO+jplnnh5uhP1RqKAxxFJUn3gzoSQKVFR4GiQ/SwpyU X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 03/04/2024 09:24, David Hildenbrand wrote: > On 03.04.24 10:18, Ryan Roberts wrote: >> On 02/04/2024 22:29, Barry Song wrote: >>> On Wed, Apr 3, 2024 at 7:46 AM David Hildenbrand wrote: >>>> >>>> On 28.03.24 10:51, Barry Song wrote: >>>>> From: Barry Song >>>>> >>>>> Profiling a system blindly with mTHP has become challenging due >>>>> to the lack of visibility into its operations. Presenting the >>>>> success rate of mTHP allocations appears to be pressing need. >>>>> >>>>> Recently, I've been experiencing significant difficulty debugging >>>>> performance improvements and regressions without these figures. >>>>> It's crucial for us to understand the true effectiveness of >>>>> mTHP in real-world scenarios, especially in systems with >>>>> fragmented memory. >>>>> >>>>> This patch sets up the framework for per-order mTHP counters, >>>>> starting with the introduction of alloc_success and alloc_fail >>>>> counters.  Incorporating additional counters should now be >>>>> straightforward as well. >>>>> >>>>> The initial two unsigned longs for each event are unused, given >>>>> that order-0 and order-1 are not mTHP. Nonetheless, this refinement >>>>> improves code clarity. >>>>> >>>>> Signed-off-by: Barry Song >>>>> --- >>>>>    -v2: >>>>>    * move to sysfs and provide per-order counters; David, Ryan, Willy >>>>>    -v1: >>>>>    https://lore.kernel.org/linux-mm/20240326030103.50678-1-21cnbao@gmail.com/ >>>>> >>>>>    include/linux/huge_mm.h | 17 +++++++++++++ >>>>>    mm/huge_memory.c        | 54 +++++++++++++++++++++++++++++++++++++++++ >>>>>    mm/memory.c             |  3 +++ >>>>>    3 files changed, 74 insertions(+) >>>>> >>>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h >>>>> index e896ca4760f6..27fa26a22a8f 100644 >>>>> --- a/include/linux/huge_mm.h >>>>> +++ b/include/linux/huge_mm.h >>>>> @@ -264,6 +264,23 @@ unsigned long thp_vma_allowable_orders(struct >>>>> vm_area_struct *vma, >>>>>                                          enforce_sysfs, orders); >>>>>    } >>>>> >>>>> +enum thp_event_item { >>>>> +     THP_ALLOC_SUCCESS, >>>>> +     THP_ALLOC_FAIL, >>>>> +     NR_THP_EVENT_ITEMS >>>>> +}; >>>> >>>> I'm wondering if these should be ANON specific for now. We might want to >>>> add others (shmem, file) in the future. >>> >>> I've two ways to do that >>> 1. rename to ANON_THP_ALLOC, so that I can have SHMEM_THP_ALLOC, FILE_THP_ALLOC >>> in the future; >>> 2. let THP_ALLOC cover all of shmem, file and anon. >>> >>> following vmstat, actually 1 might be better as we have both THP_FAULT_ALLOC and >>> THP_FILE_ALLOC for pmd-mapped THP. >>> >>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE >>>                  THP_FAULT_ALLOC, >>>                  THP_FAULT_FALLBACK, >>>                  THP_FAULT_FALLBACK_CHARGE, >>>                  THP_COLLAPSE_ALLOC, >>>                  THP_COLLAPSE_ALLOC_FAILED, >>>                  THP_FILE_ALLOC, >>>                  THP_FILE_FALLBACK, >>>                  THP_FILE_FALLBACK_CHARGE, >>>                  THP_FILE_MAPPED, >>>                  THP_SPLIT_PAGE, >>>                  THP_SPLIT_PAGE_FAILED, >>>                  THP_DEFERRED_SPLIT_PAGE, >>>                  THP_SPLIT_PMD, >>>                  THP_SCAN_EXCEED_NONE_PTE, >>>                  THP_SCAN_EXCEED_SWAP_PTE, >>>                  THP_SCAN_EXCEED_SHARED_PTE, >>> #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD >>>                  THP_SPLIT_PUD, >>> #endif >>>                  THP_ZERO_PAGE_ALLOC, >>>                  THP_ZERO_PAGE_ALLOC_FAILED, >>>                  THP_SWPOUT, >>>                  THP_SWPOUT_FALLBACK, >>> #endif >>> >>> And reading mm/shmem.c, obviously, shmem is using THP_FILE_ALLOC. >>> >>> I will rename it to ANON_THP_ALLOC in v3, let me know if you disagree :-) >> >> I don't think the name of the enum is important - its an implementation detail >> that can be changed. Its the name of the sysfs file that matters. Although of >> course its nice to keep them in sync from a maintenance pov. > > Jup. > >> >> Currently they are called "alloc_success" and "alloc_fail" I believe? Perhaps >> "anon_alloc" and "anon_alloc_fallback" are a bit more in keeping with vmstat? >> >> I'm assuming that: >> >> vmstat:thp_fault_alloc == hugepages-2048kB/stats/anon_alloc >> vmstat:thp_fault_alloc_fallback == hugepages-2048kB/stats/anon_alloc_fallback > > Or an "anon" subdirectory ... not sure, just a thought. I have no strong opinion. I'm thinking about how to easily display all the information though: $ for f in /sys/kernel/mm/hugepages/hugepages-2048kB/*; do printf '%s: ' "$f"; cat "$f"; done /sys/kernel/mm/hugepages/hugepages-2048kB/free_hugepages: 0 /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages: 0 /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages_mempolicy: 0 /sys/kernel/mm/hugepages/hugepages-2048kB/nr_overcommit_hugepages: 0 /sys/kernel/mm/hugepages/hugepages-2048kB/resv_hugepages: 0 /sys/kernel/mm/hugepages/hugepages-2048kB/surplus_hugepages: 0 $ for f in /sys/kernel/mm/hugepages/hugepages-2048kB/*; do printf '%s: ' `basename "$f"`; cat "$f"; done free_hugepages: 0 nr_hugepages: 0 nr_hugepages_mempolicy: 0 nr_overcommit_hugepages: 0 resv_hugepages: 0 surplus_hugepages: 0 It looks cleaner to me if we have all the info in the filename so we don't have to display the whole directory hierachy. But I'm sure someone more competant with bash will tell me exactly how to do it with fewer chars and an even nicer display...