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 D314ECD1284 for ; Fri, 5 Apr 2024 02:58:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 659B76B00C8; Thu, 4 Apr 2024 22:58:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 60A396B00C9; Thu, 4 Apr 2024 22:58:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4A9F96B00CA; Thu, 4 Apr 2024 22:58:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 2C27A6B00C8 for ; Thu, 4 Apr 2024 22:58:12 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id A263340CEE for ; Fri, 5 Apr 2024 02:58:11 +0000 (UTC) X-FDA: 81973969182.21.3F13452 Received: from mail-vs1-f41.google.com (mail-vs1-f41.google.com [209.85.217.41]) by imf18.hostedemail.com (Postfix) with ESMTP id 05DF51C0013 for ; Fri, 5 Apr 2024 02:58:09 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=lTyoTyF1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf18.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.41 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712285890; 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:dkim-signature; bh=7okaox6bfiyDdKaNl18nE6i5DkpCAFOq0PgyoOeEVLs=; b=kMsGdwh2qWMX/YQ1hDtbrTQL8J/7xa1J6uYUwb1vEgZcwhJQN7UVH9UfEkjE8cjhVdZaaB II/IOnAe9HtLsRuQ3SlFXOkr4M2l+tQZKj253NVY9o3dLLlz3HOHbbqJmcweRYj+qPAUm+ /yZCrFwe8kbj6ZhE6gnsq1XEjzF48jI= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=lTyoTyF1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf18.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.41 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712285890; a=rsa-sha256; cv=none; b=iO/jQkrJHT7RCRP3Ye9ZEtV8PvQEKLYtvXoNImPtCIi8pOZqsLenHw2ni5qFHPXqd6c71N 45sNoTrU7MINJ2RByyI0ae7KePe74Q7/+JJO2cswOf5k1Tum5Cx3jRio5tSf5JzpuVZf/R 1B6gmDyauaWETs0ykYAGxpVQ1hLiqEs= Received: by mail-vs1-f41.google.com with SMTP id ada2fe7eead31-4785c788ba6so610981137.0 for ; Thu, 04 Apr 2024 19:58:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712285889; x=1712890689; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=7okaox6bfiyDdKaNl18nE6i5DkpCAFOq0PgyoOeEVLs=; b=lTyoTyF16RhwkPfeJXM98+fOVLODMn+LxljQWh4F/J/SQS3p5hPSbyCYkKtdF7jq+C 2fuFTeEMle0ZhA9zGo564f34eOwq5uWCHIOyp/MEYuH0LwVR1rVcSPfd/W9yZfC/H87d G1kXqwEWvF8YMIS4qWDxPnbiG18Ma96e2rsthcIAnf7ZOLWrooT4oAmEeNpfrwFqOzmS vL/5PL9I3LmGlNYOMJEszIuNtHAYD9O58BrY3g1sigm9wpyPiIxqlkd5ntfSq4/VQSiy V8pdkF+xLqqj41N2GJTh4mEyiAEdiYmyAf9RwhJpMkxc3P4VWuYpY8f2JjIxYu3cBZjx qVhQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712285889; x=1712890689; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7okaox6bfiyDdKaNl18nE6i5DkpCAFOq0PgyoOeEVLs=; b=nKIKw76+rAxVTJU3tfspGR/rDUFjHyLKMuIqKkQu7pz/kToSslmeIH+lkBC8JZ4IaZ 5dNL9m1DpACxrIcmSSVDihVMt1o3+1VQUFhWnsmLjQGLqN1iLArlSHf0TNhXKI7LncVj iUOxkD3hMjPUxadlg0whuMthltMmDdvM1mQGU61xmAtky2utt+IymCE+KZKq0PzW4H4t 8YUu2J32Ruq5u5mmyz3Lk/9bkKDHyWmHgKwZrOMmZEyUdLcDkZCG60Jupd35cmJCne7+ 4wfCH9+U/zZ+gL3PtSOfR/DYveWA7l8NwvNfYLGECVFKidHf/K7sjoZiuHso5hZeHXJF 947A== X-Forwarded-Encrypted: i=1; AJvYcCUeftA94TBh2si2n1P9aeoBr7GFfw0f6ILOHJmzJcMGKQ1k6zwD0RrHjtKDoubhvIBlHitTzqSVDkLkSlJjIJy2hXM= X-Gm-Message-State: AOJu0YyUASSeIzvNHQ5BrR2amAUvokKxsG1ecRMZt4NRnpZKQeb/3Gmy WW+hVXJK0JKFRFkev+MXql7NIIZr0x4G53xbvoCWlAyXXDxsOq6Aqrqzdic7uAvnpSphLPkT/4j 7LSwFhYIKPuOhD3CyTs3+fMyPOAI= X-Google-Smtp-Source: AGHT+IECacoinQPsR3lCFAFGFQlIQifvO6J94M06v0vm7YnPvcitI8jCsk+mJa7j7LbZU8RL1skxThehqW59o2NR6Ts= X-Received: by 2002:a05:6102:32d0:b0:479:de59:2fc3 with SMTP id o16-20020a05610232d000b00479de592fc3mr34680vss.2.1712285888947; Thu, 04 Apr 2024 19:58:08 -0700 (PDT) MIME-Version: 1.0 References: <20240403035502.71356-1-21cnbao@gmail.com> <30392471-71f9-4eb1-8855-d9c12499346f@redhat.com> In-Reply-To: <30392471-71f9-4eb1-8855-d9c12499346f@redhat.com> From: Barry Song <21cnbao@gmail.com> Date: Fri, 5 Apr 2024 15:57:57 +1300 Message-ID: Subject: Re: [PATCH v3] mm: add per-order mTHP alloc_success and alloc_fail counters To: David Hildenbrand Cc: Ryan Roberts , akpm@linux-foundation.org, linux-mm@kvack.org, cerasuolodomenico@gmail.com, chrisl@kernel.org, kasong@tencent.com, peterx@redhat.com, surenb@google.com, v-songbaohua@oppo.com, willy@infradead.org, yosryahmed@google.com, yuzhao@google.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 05DF51C0013 X-Stat-Signature: 6j37uy3ke9hs4m1wf1a6mza5x8hr4whq X-HE-Tag: 1712285889-57373 X-HE-Meta: U2FsdGVkX1/itUHlYTzMRu3rLMmpdMLXF6tfwzJy+5RJIkaZt2E2b/hKmCgbespMa6h1f/eG/tLfEATKqFyV6cImwt1PvU8Wbbc4zoaYajgHeizk8HIAWgChLVZM4SfiQhcmtdiB7FZX6R+cfCQYihysQL26y6H6D6YyH6ED+Kx/fQse8cS+GZNTYFD6rBkVe3VpfC7JHFKO37PQiPVlv4i4afQqIfohmWvXnYoiPRF0pA7XE6wBUfbSBKl+Dlj8FjQnhUot/2ig5Ijti6fVzCdRnRPycMUso2ZwQFlPF+IaMwFr6Fkgdpvu5UykACqOG0XuuRyajXfX/+qDjvk2yzEJAs1W6AGChlZd/LBod2ImygdVlacnJBeKAtNewWqSsP89cNrt63omyaMa4BqwSN53BwFCOtizdK9q3tEKAdVHKckiAXvSMDlA9DFTfYW9jcXb4JIToHQyLTG7TKwQIlQn5QEDL2OHjF9JWJCiF8qy1JkmO8tFeg/ScdKC7VmcYqpN6NTZCjaofGtKg3IVKafHjanG+emNH6BddlEskbbOA7nTbIQF65Eyl7UnkwlPSEhP6LM26yW9Jx0MHKr9hp7kX6eVGG5t0680xDMCLv03YLVxLwtnaC9TLx1picPJhytv0J3tv0IVGD5jyyfnLqKM/XK6KdFsaAIrado9tkngw0WFn2HRgeDHiloX9UcFg6RZ4/iBhyly7ZBkFsG0SeCGX2S9FyBQDhCOBAAJORQHnXVKtM2m/z9ctEiXyiFLcGskkFGbY0JprtsM2ump9ok2ML9G/xK+aLN0IWZhLjw+afaAn5aHL0fhW/MqZFMxUO16sVS+5K6FgYs1LRldNoky4x8YMJrCoMfxtHPdyJoBCX1pCDrlUeRM3zJmbLX8bc2RIsaMR8UjgYv5c15P3Csh5DikUejVTx6JY/UFUMGHN/jIjqL1OPcrC3eZOvWfaAchsrXCLcljG9A+Mxg /1mL+uaq HXSYEEdRQpLmqWvT4Sye6jrjGcOPtkjZm69k0WUbUR6my8ejYb2H7gfo2WISIsGj8B577fub8L8hYsptiPtoI1CfXLw== 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 Fri, Apr 5, 2024 at 4:31=E2=80=AFAM David Hildenbrand = wrote: > > On 04.04.24 09:21, Ryan Roberts wrote: > > On 03/04/2024 22:00, Barry Song wrote: > >> On Thu, Apr 4, 2024 at 12:48=E2=80=AFAM Ryan Roberts wrote: > >>> > >>> On 03/04/2024 09:22, David Hildenbrand wrote: > >>>> On 03.04.24 05:55, 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 anon_alloc_success and > >>>>> anon_alloc_fail counters. Incorporating additional counters > >>>>> should now be straightforward as well. > >>>>> > >>>>> Signed-off-by: Barry Song > >>>>> --- > >>>>> -v3: > >>>>> * save some memory as order-0 and order-1 can't be THP, Ryan; > >>>>> * rename to anon_alloc as right now we only support anon to addr= ess > >>>>> David's comment; > >>>>> * drop a redundant "else", Ryan > >>>>> > >>>>> include/linux/huge_mm.h | 18 ++++++++++++++ > >>>>> mm/huge_memory.c | 54 +++++++++++++++++++++++++++++++++++= ++++++ > >>>>> mm/memory.c | 2 ++ > >>>>> 3 files changed, 74 insertions(+) > >>>>> > >>>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > >>>>> index e896ca4760f6..5e9af6be9537 100644 > >>>>> --- a/include/linux/huge_mm.h > >>>>> +++ b/include/linux/huge_mm.h > >>>>> @@ -70,6 +70,7 @@ extern struct kobj_attribute shmem_enabled_attr; > >>>>> * (which is a limitation of the THP implementation). > >>>>> */ > >>>>> #define THP_ORDERS_ALL_ANON ((BIT(PMD_ORDER + 1) - 1) & ~(BIT= (0) | BIT(1))) > >>>>> +#define THP_MIN_ORDER 2 > >>>>> /* > >>>>> * Mask of all large folio orders supported for file THP. > >>>>> @@ -264,6 +265,23 @@ unsigned long thp_vma_allowable_orders(struct > >>>>> vm_area_struct *vma, > >>>>> enforce_sysfs, orders); > >>>>> } > >>>>> +enum thp_event_item { > >>>>> + THP_ANON_ALLOC_SUCCESS, > >>>>> + THP_ANON_ALLOC_FAIL, > >>>>> + NR_THP_EVENT_ITEMS > >>>>> +}; > >>>> > >>>> Maybe use a prefix that resembles matches the enum name and is "obvi= ously" > >>>> different to the ones in vm_event_item.h, like > >>>> > >>>> enum thp_event { > >>>> THP_EVENT_ANON_ALLOC_SUCCESS, > >>>> THP_EVENT_ANON_ALLOC_FAIL, > >>>> __THP_EVENT_COUNT, > >>>> }; > >>> > >>> FWIW, I'd personally replace "event" with "stat". For me "event" only= ever > >>> increments, but "stat" can increment and decrement. An event is a typ= e of stat. > >>> > >>> You are only adding events for now, but we have identified a need for= inc/dec > >>> stats that will be added in future. > >> > >> What about the below? > >> > >> enum thp_stat { It seems we still need to use enum thp_stat_item rather than thp_stat. This follows enum zone_stat_item enum numa_stat_item enum node_stat_item And most importantly, the below looks much better enum thp_stat_item { THP_STAT_ANON_ALLOC, THP_STAT_ANON_ALLOC_FALLBACK, __THP_STAT_COUNT }; struct thp_state { unsigned long state[PMD_ORDER + 1][__THP_STAT_COUNT]; }; DECLARE_PER_CPU(struct thp_state, thp_states); than enum thp_stat { THP_STAT_ANON_ALLOC, THP_STAT_ANON_ALLOC_FALLBACK, __THP_STAT_COUNT }; struct thp_state { unsigned long state[PMD_ORDER + 1][__THP_STAT_COUNT]; }; > >> THP_EVENT_ANON_ALLOC, > >> THP_EVENT_ANON_ALLOC_FALLBACK, > >> THP_EVENT_SWPOUT, > >> THP_EVENT_SWPOUT_FALLBACK, > >> ... > >> THP_NR_ANON_PAGES, > >> THP_NR_FILE_PAGES, > > > > I find this ambiguous; Is it the number of THPs or the number of base p= ages? > > > > I think David made the point about incorporating the enum name into the= labels > > too, so that there can be no namespace confusion. How about: > > > > __ > > > > So: > > > > THP_STAT_EV_ANON_ALLOC > > THP_STAT_EV_ANON_ALLOC_FALLBACK > > THP_STAT_EV_ANON_PARTIAL > > THP_STAT_EV_SWPOUT > > THP_STAT_EV_SWPOUT_FALLBACK > > ... > > THP_STAT_NR_ANON > > THP_STAT_NR_FILE > > ... > > __THP_STAT_COUNT > > I'd even drop the "EV". "NR_ANON" vs "ANON_ALLOC" etc. is expressive enou= gh. ok. > > -- > Cheers, > > David / dhildenb Thanks Barry