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 058E8CD11C2 for ; Fri, 5 Apr 2024 04:01:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 332CA6B00D8; Fri, 5 Apr 2024 00:01:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2E3036B00D9; Fri, 5 Apr 2024 00:01:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1D22F6B00DA; Fri, 5 Apr 2024 00:01:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id F04DC6B00D8 for ; Fri, 5 Apr 2024 00:01:20 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 63F14C0A9C for ; Fri, 5 Apr 2024 04:01:20 +0000 (UTC) X-FDA: 81974128320.10.EC8AC5E Received: from mail-ua1-f53.google.com (mail-ua1-f53.google.com [209.85.222.53]) by imf05.hostedemail.com (Postfix) with ESMTP id 818DE10001F for ; Fri, 5 Apr 2024 04:01:17 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=dXNBOGT7; spf=pass (imf05.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.53 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712289677; 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=Yn5+uPM1ERH0pDkmU3Tel1mPgcgDqdpVpZAfWRzm9AA=; b=pIdGqFdD8Qh5TxAXC3RxuHUXz6tb3I+W0hfPKc4q76wdWY7bUS9BziZMlIL+K8lAh/LsjQ YKMdMwaKaFKI1tY2jQWZM0Wzyivon0iULmYxtGTotAEjaKx8dHQyHNBUnNaZmvkgUOB/D1 mBEskFCCBRrDtW8nnTUfBeXHi5p1KvQ= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=dXNBOGT7; spf=pass (imf05.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.53 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712289677; a=rsa-sha256; cv=none; b=Pk4nv6i7rp10Tiw1IuAmdRf1q1TFpcTUxyP5FDOaFBvEXqRdSfLt8O91f7Vl0Zd0Lyw2tQ zxGpmAdhaRNhkBKL7DzFCWBINI/8wueYjOcu6qk2gp+lTnTS6K2cBPjv+2yRUR2nn5BdIF YrZaVilVuPBh4mlb6mNVPH3YXL7ZKGk= Received: by mail-ua1-f53.google.com with SMTP id a1e0cc1a2514c-7db36dbd474so547789241.2 for ; Thu, 04 Apr 2024 21:01:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712289676; x=1712894476; 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=Yn5+uPM1ERH0pDkmU3Tel1mPgcgDqdpVpZAfWRzm9AA=; b=dXNBOGT7UeM4HBGHDPO30VmOrogkdoxOGBIMFkxqmuvYwq4y11XFn8hMqYa3s3z0IB Ljvbs98weze3WVNEI2/SrIgIhfoFupPMFRItmgcwIlGtKj3Fpo00mqc6Yb7oDSBoHV/w 6SOA7eRLvshuMSxUztz61d8a3wGJM0NqdyeKxzX9RnHPmr3Rr8WVwohdeQqJ5PMjxVon uLoh7SakvIC3Xg70Ete8R2TbP6NZ75wgNTDxLIgfKTA6cJ9oJhezqXvtzbMcC5N92SsN HV1F2QYe0Unv9wcvdc+6AxpF5QUPmd/+Q1xRdvGlO5B29e6bdCC1G3x6vqMMek044LVF m60Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712289676; x=1712894476; 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=Yn5+uPM1ERH0pDkmU3Tel1mPgcgDqdpVpZAfWRzm9AA=; b=ggFJCmcrMhRLIHwmMuadJFB9Goi8FsjFnQvLnCT9OHP7jx00UEVRCq+8ni/+IvKVFT r6dGTJBq2WOMiEL0VzAg3bdQSr+lLEe/QXC0s6zaGOYtBkt+Uv2B99CHUWwOlGBWCVKt cYIUobTqXdwE5VNfWKDVngGLe+vOu42lU5nS2fKrR2gCrL59ZyGwaiuhxOAnh/2v3xiL f/4XM9YeGpoZth9rTxnlXFBDWaJ3IG9m2J0ID58CGTdqHatc0XvZ65qgf8u+DVKfcmTP hiQSXkUl4I+Wt6mGFMnjGp1ccNC53t6RN3LSI6k/0kLFY+Aunepbb3SAUx5HPFCSsRBE EMJA== X-Forwarded-Encrypted: i=1; AJvYcCVC5rfMF6ZpNGCGHY+f8HQXQwRe7yIpKO4i5cp40J+JiD21/lhX/Pdts7eiOwOCRYJmahfYQD3/LysbtM3kp3fG2rc= X-Gm-Message-State: AOJu0YwbIkcdekc5SrcRA6LxyA36MtE+t8yqWi0pnD9K2Nxib9WoRAYu toqZkuZQkRwzHPsEYLqWEzg/yW8oKw4RNeuTgqqA6wAMDGgj9faSmLqBtETGQGots2LWJ5C8jr4 BgGMlflRXYQ61f2wWxq3Kw9hW+vk= X-Google-Smtp-Source: AGHT+IEtiYqWH6KynFiHEBwI5vkBEpHCI3sTjpe9LmzMJHmwOJAbCvXiOs3v2TbOM8pB84pEsQWL+Eb5n3xCLpGbcg8= X-Received: by 2002:ac5:c929:0:b0:4d4:126b:2c8 with SMTP id u9-20020ac5c929000000b004d4126b02c8mr382182vkl.9.1712289676573; Thu, 04 Apr 2024 21:01:16 -0700 (PDT) MIME-Version: 1.0 References: <20240403035502.71356-1-21cnbao@gmail.com> <30392471-71f9-4eb1-8855-d9c12499346f@redhat.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Fri, 5 Apr 2024 17:01:05 +1300 Message-ID: Subject: Re: [PATCH v3] mm: add per-order mTHP alloc_success and alloc_fail counters To: David Hildenbrand , Ryan Roberts Cc: 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-Rspamd-Queue-Id: 818DE10001F X-Rspam-User: X-Stat-Signature: ijacqf379b6mqipigkd4pd1yoyehq49x X-Rspamd-Server: rspam01 X-HE-Tag: 1712289677-534150 X-HE-Meta: U2FsdGVkX1+DqbkuyYN63dT9J2aJdC9gH5rZjM/MiTJG5soOJ+WSHS7Hu4qm5nZp4VbWo6wGxQrbXuwxvYRttmORE6GnEAVE1gUxlPOSFbrqhT+5y4WJFPsq4XQb0tZIQL/f4p5hkWkFMY7nOrEsSERxmq/vEfiBsjVfygJ11dvLYigrN+VqWDU6MIYoSERuDFE+4vGZaQHsicU8XN9IgObZ1aNlWjvf3ay8ZE/JiIQ58G8677siT1WUSkFfio+rWw25eRQWZiXaMm9YvJrUANVOb27lILFNI2gr0YrI2zsrT6ug1L81XHNdOoOZ5IbWeCVxbx5QkO8UyWNFLIR4QY9nSVeJ9Ivm0ltETw7O1mk9Dr6zMyCbo7f+7IHszU72ow4sBgA8rlvwaVjG06ICaqVX+BSsTYZMBH7SrTKFidmq0K66NWufUipet6XCffj1Y1eCtUDookJk5yBHjMk01+wDbSubPFevT+ERe2qOwhoARS3OQfDN7fx+9YzPJpUWa0tsfBBcyvWR/OrpyA3+BFi9ie3+BPd/SRC0tIZ6lvPiAnnukq79feBKji2por/KuylO85RvvavOPDPoQrpmLMg46WGxgDJDtQ5zXIYkdaWylrLaA9bcGC1CEuf22AKSZloBvdZdrrYzOHx8RmHygOj7qgJbPk+ngz2YmShS/bEIMOOgRFtPm6PRlJWHv9kORgdKUHbU/9xvK8yVYLZ+51BY4bnPG5g27VbHyEfdVgomW0lRunYjpkYrAPlAZy1G/XxPH17NAvM8p+pksRGce1ls92q1b+QaBJn0uzW8LUpSELus4atjsuJ362J7EHkuTpgqieTrIZGMVGOi8ZvWrAZguAxv+klfUk9lbvbfe8GCjph5kqzPN742HOKtwp5L1Rt+jcaJYCbpwWq7AK9gb5nQoeI7uFr7HhkZRqOOuP/jrmbyQGqX3Ww+62ESJ7Y6mWGPYYAEamCtMscXBhK 4oPg9lN5 xGI5gLSWxPvEadNq+ayssFUCzcuLAMNHPjcx9nVZ69IsK/JJhcerp4foRtwuwA9H0n1y73EfeLbIStyMtdxEdBul+Q4vFaMLFBrdiBwO3//Ek5MjDgsFn/Iw4dH96VB8fijQf5XENxbp0uoHSTclPXuR8ZUTcs8re8k+DBMMpkxv/HNswL7UfZbzQEMdk9Lqoz1tAQTTKRuqXrZiLW5ucG6NmYvhZk4XZyTYREqXvF7EotXcbO75U9PLlYIDyfSCLGhkPLN1YgxDOsIiQcfLc7/TSsxg3mteirgcXAtwx7q1sT8ZXZgwjn+1K4FG1JQi6Mo4zP6HmZ94hXWIwY4toKxKSP7jNTDEQvDryhnEhgaZHN3rgMAiHjvTi0dxYcKf8I/g4mMBHXly5kPEMpjDRqtBbevAMKmTcApLBjDx9FMHmZO81nChepPE9DuAvmgmYkrXt+nIcBbz28l5ILO6hdtbKradeVmI2WpwgJPWajLOzl6iEu17m0gBHzjxe4l6ZChGnVhEjOvyaGeCPaUKI+PJ5ekoJ58eDVX+07CLUGIE9TYSP0yHK4A0HWGqQ7qBdxXF/ 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 3:57=E2=80=AFPM Barry Song <21cnbao@gmail.com> wrote= : > > 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 ad= dress > > >>>>> 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) & ~(B= IT(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(struc= t > > >>>>> 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 "ob= viously" > > >>>> 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" on= ly ever > > >>> increments, but "stat" can increment and decrement. An event is a t= ype of stat. > > >>> > > >>> You are only adding events for now, but we have identified a need f= or 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= pages? > > > > > > I think David made the point about incorporating the enum name into t= he 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 en= ough. > > ok. Hi David, Ryan, I've named everything as follows. Please let me know if you have any furthe= r suggestions before I send the updated version :-) diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index e896ca4760f6..cc13fa14aa32 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_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); + +static inline void count_thp_state(int order, enum thp_stat_item item) +{ + this_cpu_inc(thp_states.state[order][item]); +} + #define transparent_hugepage_use_zero_page() \ (transparent_hugepage_flags & \ (1<state[order][item]; + } + + return sum; +} + +#define THP_STATE_ATTR(_name, _index) \ +static ssize_t _name##_show(struct kobject *kobj, \ + struct kobj_attribute *attr, char *buf) \ +{ \ + int order =3D to_thpsize(kobj)->order; \ + \ + return sysfs_emit(buf, "%lu\n", sum_thp_states(order, _index)); \ +} \ +static struct kobj_attribute _name##_attr =3D __ATTR_RO(_name) + +THP_STATE_ATTR(anon_alloc, THP_STAT_ANON_ALLOC); +THP_STATE_ATTR(anon_alloc_fallback, THP_STAT_ANON_ALLOC_FALLBACK); > > > > > -- > > Cheers, > > > > David / dhildenb Thanks Barry