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 66282C4345F for ; Fri, 12 Apr 2024 09:27:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EE0B06B0092; Fri, 12 Apr 2024 05:27:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E913B6B0093; Fri, 12 Apr 2024 05:27:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D587D6B0095; Fri, 12 Apr 2024 05:27:06 -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 B6B696B0092 for ; Fri, 12 Apr 2024 05:27:06 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 53F70A0D86 for ; Fri, 12 Apr 2024 09:27:06 +0000 (UTC) X-FDA: 82000350852.20.1E0B0ED Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf06.hostedemail.com (Postfix) with ESMTP id 8BC36180002 for ; Fri, 12 Apr 2024 09:27:04 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf06.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=1712914024; 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=81qWZbeq1CSc4Z1QezEQ4/03Nl0bQvweoSZaqSScU2k=; b=fVI2vSGTKn9p87mlIGQnbno0pN414HXF1op1K/0KnWlmL4L9WdWH/I9TA2VT174srG39C+ FmW8JiDsg59EYR4BHKNbyjMRcB/URL0m1/2UBy2AQ03NMIKpJEHoK3/+jHUDaYT0IKoVuv zvLg0+F30yVERI/1PIU7Bz1bNAOnQ/Y= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf06.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=1712914024; a=rsa-sha256; cv=none; b=fPQLpt47OPkiMltuPHlB3vphb4E8C2usaho0Be85gP0egs/FVfWMkDFbHqtCMUhFAbyAuB 5l5Oowsna5DXs9nOJSTAEVM7pxuWm7iURuZsNAQcmd8yFCB6qrcvxID8xJn+nxugfmVrri nCh2iGpqeaS8ZsMbgpXRbL7kE98YfYw= 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 D68F7339; Fri, 12 Apr 2024 02:27:32 -0700 (PDT) Received: from [10.57.73.208] (unknown [10.57.73.208]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C8ECE3F64C; Fri, 12 Apr 2024 02:27:00 -0700 (PDT) Message-ID: Date: Fri, 12 Apr 2024 10:26:58 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 1/4] mm: add per-order mTHP anon_fault_alloc and anon_fault_fallback counters Content-Language: en-GB To: Barry Song <21cnbao@gmail.com>, akpm@linux-foundation.org, linux-mm@kvack.org Cc: cerasuolodomenico@gmail.com, chrisl@kernel.org, david@redhat.com, kasong@tencent.com, linux-kernel@vger.kernel.org, peterx@redhat.com, surenb@google.com, v-songbaohua@oppo.com, willy@infradead.org, yosryahmed@google.com, yuzhao@google.com, corbet@lwn.net References: <20240412073740.294272-1-21cnbao@gmail.com> <20240412073740.294272-2-21cnbao@gmail.com> From: Ryan Roberts In-Reply-To: <20240412073740.294272-2-21cnbao@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 8BC36180002 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: cjwxgypx3tcagbgshjhcjswb3csupajg X-HE-Tag: 1712914024-892198 X-HE-Meta: U2FsdGVkX19xoEU2ni7IC3PSODOEHjIEDhpZ3AK8Cwa/ktKzaygW74B4rf62pDAfA99/Hi++YBXaSG+8CAB5yNPfM/0ztqC1vOtQRpVrqvHMAufuTo+CWpnaUXDjY6tXyHNLPK1EtGSt0NdzqHJj/a/zrmoHLOdBF6kdmgqr1FzNUtR0tR0xqEHeaXev3H8CQvvRJA+9UUw+6l6tjbY8nTHIle9yN4OtDhpQHaQKQIRsesuEpjWvbfLOD98pvfXXLiaGyyZqrQ1b71vecvBdbkCdL9ykaiX4tlmbUULU3ZxC02jB+sQ5/INPAIuTyJs8gBENBjFGhDj6/UZSrMHBx0bpxl1WQ20mOM7vRDJesokguMSk8rHZh4K25zLl6CqS42nUnVn0pmM6ppz5HWyWtMk5l+c2M+wq6Hc7vqSO3E05qXsd7ZcDg9YGU6W56yK5uZH46T1lbFw9RcHGO4ophwiuLFh3UG1RlJ+81vFeMZNs+iU/S+HjzaiJNT9XSFqMh7Z51MpF3rkFvUZMPfTWH4UjNgL0+784emnAG52CPhz2Q4GDMhQw98JCUWCz9p2cmXvpeEscC093anCKeJRWZASpCn5lfl/ZKmHmduiRrPH+iYe/zf+4OA3nuJLcUILr9K8Ty7zeC3ezUKK62YHXZ806TOzlWMz6McYvVOxo5y6Nk3UsVg5ZLAghop0laKUwdHezQzGUNB2m2aWnLUVNK1t3D1ZAeZb8ZRhvxq9gZy2MvZtgseXewR2KTl+ighakNnRHJeGB5sznbACE9PFt7IiI82iY6ueNeQOk2K75SHt4hYBo5zgrbpFSL0xSerSFKhrJT/IbOCbsCJCvEgigb3JI6ELd05ygav3eIUTihClpaqYhN0bGjzN9qPG6/s5zHsPlyg68iF4D3bL0raY6HacjEKlanMA1EMCOEfDpmFlfig+5SAdeHLyZLRoOh/REhxk704DSTunQxUOZHsf 9WRiR5cH pGZM8xD8JVHotU0pTfojxya4FMPCIo2o18N/fTem9nWHnShum1QyK2drI1qyPPxjzEA57QUm2anKhcTFAmzocYS78e0fzdvn2Dw8nML3VkYHcsCiOqfEc2inyJUnucpUcOLWlEEGGsv9ok4YwItpiTHrbEtZCtUI3wB6iEA4m0Xw21Ow2g7lh5rUMd/H9tsTFBdPzIkT6W6kqYaK5K+yaEliO8Ntcmr0oJp0uG4lVsysKEEMTCc0sfMQFXz9/22dmtmZFOvjMI1NTpd+P2a9LBOhTMKidV5ib72cKDW/6/qrDiN0YFe8fy9F9lBvYo6o7pBPZoWA02OJu351sQmFrFwVkJ8cXkE2EJqWFokj4iZLE92CXdSXZhExfdFK/prjpKoFF5bKoLrNe9oWYU7xkmiyQKZ/dExCG5mjDumYwUYpQB34Wk4cnqfqquK7cgqq/YGQ/YhW9QhTd760aSM5o4QUzGQoy1ZxxWqG76Lr7TsFQytg2Qruz9ftMnhsHCG2Af+XMF81Xqs0co+w+wkW42OMcoDpFMam/SgilIhvSz6Ap1/hLNwUutI0ITg== 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: Hi Barry, 2 remaining comments - otherwise looks good. (same comments I just made in the v4 conversation). On 12/04/2024 08:37, 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 establishes the framework for per-order mTHP > counters. It begins by introducing the anon_fault_alloc and > anon_fault_fallback counters. Additionally, to maintain consistency > with thp_fault_fallback_charge in /proc/vmstat, this patch also tracks > anon_fault_fallback_charge when mem_cgroup_charge fails for mTHP. > Incorporating additional counters should now be straightforward as well. > > Signed-off-by: Barry Song > Cc: Chris Li > Cc: David Hildenbrand > Cc: Domenico Cerasuolo > Cc: Kairui Song > Cc: Matthew Wilcox (Oracle) > Cc: Peter Xu > Cc: Ryan Roberts > Cc: Suren Baghdasaryan > Cc: Yosry Ahmed > Cc: Yu Zhao > --- > include/linux/huge_mm.h | 51 ++++++++++++++++++++++++++++++++++ > mm/huge_memory.c | 61 +++++++++++++++++++++++++++++++++++++++++ > mm/memory.c | 3 ++ > mm/page_alloc.c | 4 +++ > 4 files changed, 119 insertions(+) > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > index e896ca4760f6..c5beb54b97cb 100644 > --- a/include/linux/huge_mm.h > +++ b/include/linux/huge_mm.h > @@ -264,6 +264,57 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma, > enforce_sysfs, orders); > } > > +enum mthp_stat_item { > + MTHP_STAT_ANON_FAULT_ALLOC, > + MTHP_STAT_ANON_FAULT_FALLBACK, > + MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE, > + __MTHP_STAT_COUNT > +}; > + > +struct mthp_stat { > + unsigned long stats[0][__MTHP_STAT_COUNT]; > +}; > + > +extern struct mthp_stat __percpu *mthp_stats; > + > +static inline void count_mthp_stat(int order, enum mthp_stat_item item) > +{ > + if (order <= 0 || order > PMD_ORDER || !mthp_stats) > + return; > + > + this_cpu_inc(mthp_stats->stats[order][item]); > +} > + > +static inline void count_mthp_stats(int order, enum mthp_stat_item item, long delta) > +{ > + if (order <= 0 || order > PMD_ORDER || !mthp_stats) > + return; > + > + this_cpu_add(mthp_stats->stats[order][item], delta); > +} > + > +/* > + * Fold the foreign cpu mthp stats into our own. > + * > + * This is adding to the stats on one processor > + * but keeps the global counts constant. > + */ > +static inline void mthp_stats_fold_cpu(int cpu) > +{ > + struct mthp_stat *fold_stat; > + int i, j; > + > + if (!mthp_stats) > + return; > + fold_stat = per_cpu_ptr(mthp_stats, cpu); > + for (i = 1; i <= PMD_ORDER; i++) { > + for (j = 0; j < __MTHP_STAT_COUNT; j++) { > + count_mthp_stats(i, j, fold_stat->stats[i][j]); > + fold_stat->stats[i][j] = 0; > + } > + } > +} This is a pretty horrible hack; I'm pretty sure just summing for all *possible* cpus should work. > + > #define transparent_hugepage_use_zero_page() \ > (transparent_hugepage_flags & \ > (1< diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index dc30139590e6..21c4ac74b484 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -526,6 +526,50 @@ static const struct kobj_type thpsize_ktype = { > .sysfs_ops = &kobj_sysfs_ops, > }; > > +struct mthp_stat __percpu *mthp_stats; > + > +static unsigned long sum_mthp_stat(int order, enum mthp_stat_item item) > +{ > + unsigned long sum = 0; > + int cpu; > + > + cpus_read_lock(); > + for_each_online_cpu(cpu) { > + struct mthp_stat *this = per_cpu_ptr(mthp_stats, cpu); > + > + sum += this->stats[order][item]; > + } > + cpus_read_unlock(); > + > + return sum; > +} > + > +#define DEFINE_MTHP_STAT_ATTR(_name, _index) \ > +static ssize_t _name##_show(struct kobject *kobj, \ > + struct kobj_attribute *attr, char *buf) \ > +{ \ > + int order = to_thpsize(kobj)->order; \ > + \ > + return sysfs_emit(buf, "%lu\n", sum_mthp_stat(order, _index)); \ > +} \ > +static struct kobj_attribute _name##_attr = __ATTR_RO(_name) > + > +DEFINE_MTHP_STAT_ATTR(anon_fault_alloc, MTHP_STAT_ANON_FAULT_ALLOC); > +DEFINE_MTHP_STAT_ATTR(anon_fault_fallback, MTHP_STAT_ANON_FAULT_FALLBACK); > +DEFINE_MTHP_STAT_ATTR(anon_fault_fallback_charge, MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE); > + > +static struct attribute *stats_attrs[] = { > + &anon_fault_alloc_attr.attr, > + &anon_fault_fallback_attr.attr, > + &anon_fault_fallback_charge_attr.attr, > + NULL, > +}; > + > +static struct attribute_group stats_attr_group = { > + .name = "stats", > + .attrs = stats_attrs, > +}; > + > static struct thpsize *thpsize_create(int order, struct kobject *parent) > { > unsigned long size = (PAGE_SIZE << order) / SZ_1K; > @@ -549,6 +593,12 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent) > return ERR_PTR(ret); > } > > + ret = sysfs_create_group(&thpsize->kobj, &stats_attr_group); > + if (ret) { > + kobject_put(&thpsize->kobj); > + return ERR_PTR(ret); > + } > + > thpsize->order = order; > return thpsize; > } > @@ -691,6 +741,11 @@ static int __init hugepage_init(void) > */ > MAYBE_BUILD_BUG_ON(HPAGE_PMD_ORDER < 2); > > + mthp_stats = __alloc_percpu((PMD_ORDER + 1) * sizeof(mthp_stats->stats[0]), > + sizeof(unsigned long)); Personally I think it would be cleaner to allocate statically using ilog2(MAX_PTRS_PER_PTE) instead of PMD_ORDER. > + if (!mthp_stats) > + return -ENOMEM; > + > err = hugepage_init_sysfs(&hugepage_kobj); > if (err) > goto err_sysfs; > @@ -725,6 +780,8 @@ static int __init hugepage_init(void) > err_slab: > hugepage_exit_sysfs(hugepage_kobj); > err_sysfs: > + free_percpu(mthp_stats); > + mthp_stats = NULL; > return err; > } > subsys_initcall(hugepage_init); > @@ -880,6 +937,8 @@ static vm_fault_t __do_huge_pmd_anonymous_page(struct vm_fault *vmf, > folio_put(folio); > count_vm_event(THP_FAULT_FALLBACK); > count_vm_event(THP_FAULT_FALLBACK_CHARGE); > + count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_FAULT_FALLBACK); > + count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE); > return VM_FAULT_FALLBACK; > } > folio_throttle_swaprate(folio, gfp); > @@ -929,6 +988,7 @@ static vm_fault_t __do_huge_pmd_anonymous_page(struct vm_fault *vmf, > mm_inc_nr_ptes(vma->vm_mm); > spin_unlock(vmf->ptl); > count_vm_event(THP_FAULT_ALLOC); > + count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_FAULT_ALLOC); > count_memcg_event_mm(vma->vm_mm, THP_FAULT_ALLOC); > } > > @@ -1050,6 +1110,7 @@ vm_fault_t do_huge_pmd_anonymous_page(struct vm_fault *vmf) > folio = vma_alloc_folio(gfp, HPAGE_PMD_ORDER, vma, haddr, true); > if (unlikely(!folio)) { > count_vm_event(THP_FAULT_FALLBACK); > + count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_FAULT_FALLBACK); > return VM_FAULT_FALLBACK; > } > return __do_huge_pmd_anonymous_page(vmf, &folio->page, gfp); > diff --git a/mm/memory.c b/mm/memory.c > index 649a547fe8e3..06048af7cf9a 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -4368,6 +4368,7 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf) > folio = vma_alloc_folio(gfp, order, vma, addr, true); > if (folio) { > if (mem_cgroup_charge(folio, vma->vm_mm, gfp)) { > + count_mthp_stat(order, MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE); > folio_put(folio); > goto next; > } > @@ -4376,6 +4377,7 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf) > return folio; > } > next: > + count_mthp_stat(order, MTHP_STAT_ANON_FAULT_FALLBACK); > order = next_order(&orders, order); > } > > @@ -4485,6 +4487,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf) > > folio_ref_add(folio, nr_pages - 1); > add_mm_counter(vma->vm_mm, MM_ANONPAGES, nr_pages); > + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_FAULT_ALLOC); > folio_add_new_anon_rmap(folio, vma, addr); > folio_add_lru_vma(folio, vma); > setpte: > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index b51becf03d1e..3135b5ca2457 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -5840,6 +5840,10 @@ static int page_alloc_cpu_dead(unsigned int cpu) > */ > vm_events_fold_cpu(cpu); > > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > + mthp_stats_fold_cpu(cpu); > +#endif > + > /* > * Zero the differential counters of the dead processor > * so that the vm statistics are consistent.