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 4D96BC433F5 for ; Wed, 15 Dec 2021 17:16:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 98AD76B0071; Wed, 15 Dec 2021 12:16:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 913EE6B0073; Wed, 15 Dec 2021 12:16:45 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7B4166B0074; Wed, 15 Dec 2021 12:16:45 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0070.hostedemail.com [216.40.44.70]) by kanga.kvack.org (Postfix) with ESMTP id 65FFC6B0071 for ; Wed, 15 Dec 2021 12:16:45 -0500 (EST) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 24B22180CB07F for ; Wed, 15 Dec 2021 17:16:35 +0000 (UTC) X-FDA: 78920682750.29.4887C8D Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf27.hostedemail.com (Postfix) with ESMTP id 4D66840006 for ; Wed, 15 Dec 2021 17:16:34 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id AB08F619F6; Wed, 15 Dec 2021 17:16:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F1789C36AE3; Wed, 15 Dec 2021 17:16:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1639588593; bh=LSdTbxpPrL6NixMyvcEWiwYL4zMtFNdl2j/n8kK1PQ4=; h=From:To:Cc:Subject:Date:In-Reply-To:From; b=ibB1apEz+mKi0ZEAGlJc0EiMhJVCMrCKSXBsgqgI9GeB+MtOEkvajFwxb/D5g2529 PRVoPFdNmfKJlB1phkwuk/OcX1CE6UO+JNV1guSQ4Tn8DKtKl2u5rR3XSXZJ9czQu4 puzrfVZT69NextGuL63baI2hdhsx3zW1GipUEmE+F0hSJRlYRn+RomCNndt6UIzJU4 zaRZZ3lqeZkFGfdfcvwvhxGrUJLAbrYFqjCL974RaRJ2oHHGx3jWZYQCOHTs+KigfC AyTOXgdK7K6i+hshRNociH3PiRW4taAjnY8zwHV+4Iv+CuXck8RDGvjMXmmi1x+PMf DmRTHBc66R79Q== From: SeongJae Park To: Baolin Wang Cc: sj@kernel.org, akpm@linux-foundation.org, mike.kravetz@oracle.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/damon: Add access checking for hugetlb pages Date: Wed, 15 Dec 2021 17:16:30 +0000 Message-Id: <20211215171630.3899-1-sj@kernel.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <8ce19b8e22e24aa93e24bf89cd723d2e1a3250c9.1639581537.git.baolin.wang@linux.alibaba.com> X-Stat-Signature: goeft5wnfngmnogecomkqmydn94b7npz Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ibB1apEz; spf=pass (imf27.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=none) header.from=kernel.org X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 4D66840006 X-HE-Tag: 1639588594-801189 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: On Wed, 15 Dec 2021 23:23:25 +0800 Baolin Wang wrote: > The process's VMAs can be mapped by hugetlb page, but now the DAMON > did not implement the access checking for hugetlb pte, so we can not > get the actual access count like below if a process VMAs were mapped > by hugetlb. > > damon_aggregated: target_id=18446614368406014464 > nr_regions=12 4194304-5476352: 0 545 > damon_aggregated: target_id=18446614368406014464 > nr_regions=12 140662370467840-140662372970496: 0 545 > damon_aggregated: target_id=18446614368406014464 > nr_regions=12 140662372970496-140662375460864: 0 545 > damon_aggregated: target_id=18446614368406014464 > nr_regions=12 140662375460864-140662377951232: 0 545 > damon_aggregated: target_id=18446614368406014464 > nr_regions=12 140662377951232-140662380449792: 0 545 > damon_aggregated: target_id=18446614368406014464 > nr_regions=12 140662380449792-140662382944256: 0 545 > ...... > > Thus this patch adds hugetlb access checking support, with this patch > we can see below VMA mapped by hugetlb access count. > > damon_aggregated: target_id=18446613056935405824 > nr_regions=12 140296486649856-140296489914368: 1 3 > damon_aggregated: target_id=18446613056935405824 > nr_regions=12 140296489914368-140296492978176: 1 3 > damon_aggregated: target_id=18446613056935405824 > nr_regions=12 140296492978176-140296495439872: 1 3 > damon_aggregated: target_id=18446613056935405824 > nr_regions=12 140296495439872-140296498311168: 1 3 > damon_aggregated: target_id=18446613056935405824 > nr_regions=12 140296498311168-140296501198848: 1 3 > damon_aggregated: target_id=18446613056935405824 > nr_regions=12 140296501198848-140296504320000: 1 3 > damon_aggregated: target_id=18446613056935405824 > nr_regions=12 140296504320000-140296507568128: 1 2 > ...... Thank you for this patch! > > Signed-off-by: Baolin Wang > --- > mm/damon/prmtv-common.c | 37 ++++++++++++++++++++++++++++ > mm/damon/prmtv-common.h | 11 +++++++++ > mm/damon/vaddr.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 113 insertions(+) > > diff --git a/mm/damon/prmtv-common.c b/mm/damon/prmtv-common.c > index 92a04f5..155afb8 100644 > --- a/mm/damon/prmtv-common.c > +++ b/mm/damon/prmtv-common.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > > #include "prmtv-common.h" > > @@ -86,6 +87,42 @@ void damon_pmdp_mkold(pmd_t *pmd, struct mm_struct *mm, unsigned long addr) > #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > } > > +#ifdef CONFIG_HUGETLB_PAGE > +void damon_hugetlb_mkold(pte_t *pte, struct mm_struct *mm, > + struct vm_area_struct *vma, unsigned long addr) > +{ > + bool referenced = false; > + struct hstate *h = hstate_vma(vma); > + pte_t entry; > + struct page *page; > + > + entry = huge_ptep_get(pte); > + page = pte_page(entry); Could we do these assignments in above definition part like this? pte_t entry = huge_ptep_get(pte); struct page *page = pte_page(entry); > + if (!page) > + return; > + > + get_page(page); > + > + if (pte_young(entry)) { > + referenced = true; > + entry = pte_mkold(entry); > + huge_ptep_set_access_flags(vma, addr, pte, entry, > + vma->vm_flags & VM_WRITE); > + } > + > +#ifdef CONFIG_MMU_NOTIFIER > + if (mmu_notifier_clear_young(mm, addr, addr + huge_page_size(h))) > + referenced = true; > +#endif /* CONFIG_MMU_NOTIFIER */ > + > + if (referenced) > + set_page_young(page); > + > + set_page_idle(page); > + put_page(page); > +} > +#endif > + > #define DAMON_MAX_SUBSCORE (100) > #define DAMON_MAX_AGE_IN_LOG (32) > > diff --git a/mm/damon/prmtv-common.h b/mm/damon/prmtv-common.h > index e790cb5..65efcb7 100644 > --- a/mm/damon/prmtv-common.h > +++ b/mm/damon/prmtv-common.h > @@ -12,5 +12,16 @@ > void damon_ptep_mkold(pte_t *pte, struct mm_struct *mm, unsigned long addr); > void damon_pmdp_mkold(pmd_t *pmd, struct mm_struct *mm, unsigned long addr); > > +#ifdef CONFIG_HUGETLB_PAGE > +void damon_hugetlb_mkold(pte_t *pte, struct mm_struct *mm, > + struct vm_area_struct *vma, unsigned long addr); > +#else > +static inline void damon_hugetlb_mkold(pte_t *pte, struct mm_struct *mm, > + struct vm_area_struct *vma, > + unsigned long addr) > +{ > +} > +#endif > + Seems damon_hugetlb_mkold() is called from only vaddr.c. Could you move the definition into vaddr.c and remove this change in prmtv-common.h? > int damon_pageout_score(struct damon_ctx *c, struct damon_region *r, > struct damos *s); > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c > index 78ff2bc..ee116e5 100644 > --- a/mm/damon/vaddr.c > +++ b/mm/damon/vaddr.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > > #include "prmtv-common.h" > > @@ -386,8 +387,33 @@ static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr, > return 0; > } > > +#ifdef CONFIG_HUGETLB_PAGE > +static int damon_mkold_hugetlb_entry(pte_t *pte, unsigned long hmask, > + unsigned long addr, unsigned long end, > + struct mm_walk *walk) > +{ > + struct hstate *h = hstate_vma(walk->vma); > + spinlock_t *ptl; > + pte_t entry; > + > + ptl = huge_pte_lock(h, walk->mm, pte); > + entry = huge_ptep_get(pte); Could we do above assignments in the variables definitions? > + if (!pte_present(entry)) > + goto out; > + > + damon_hugetlb_mkold(pte, walk->mm, walk->vma, addr); > + > +out: > + spin_unlock(ptl); > + return 0; > +} > +#else > +#define damon_mkold_hugetlb_entry NULL > +#endif > + > static const struct mm_walk_ops damon_mkold_ops = { > .pmd_entry = damon_mkold_pmd_entry, > + .hugetlb_entry = damon_mkold_hugetlb_entry, > }; > > static void damon_va_mkold(struct mm_struct *mm, unsigned long addr) > @@ -482,8 +508,47 @@ static int damon_young_pmd_entry(pmd_t *pmd, unsigned long addr, > return 0; > } > > +#ifdef CONFIG_HUGETLB_PAGE > +static int damon_young_hugetlb_entry(pte_t *pte, unsigned long hmask, > + unsigned long addr, unsigned long end, > + struct mm_walk *walk) > +{ > + struct damon_young_walk_private *priv = walk->private; > + struct hstate *h = hstate_vma(walk->vma); > + struct page *page; > + spinlock_t *ptl; > + pte_t entry; > + > + ptl = huge_pte_lock(h, walk->mm, pte); > + entry = huge_ptep_get(pte); Could we do these assignments in the above variables definitions? Thanks, SJ > + if (!pte_present(entry)) > + goto out; > + > + page = pte_page(entry); > + if (!page) > + goto out; > + > + get_page(page); > + > + if (pte_young(entry) || !page_is_idle(page) || > + mmu_notifier_test_young(walk->mm, addr)) { > + *priv->page_sz = huge_page_size(h); > + priv->young = true; > + } > + > + put_page(page); > + > +out: > + spin_unlock(ptl); > + return 0; > +} > +#else > +#define damon_young_hugetlb_entry NULL > +#endif > + > static const struct mm_walk_ops damon_young_ops = { > .pmd_entry = damon_young_pmd_entry, > + .hugetlb_entry = damon_young_hugetlb_entry, > }; > > static bool damon_va_young(struct mm_struct *mm, unsigned long addr, > -- > 1.8.3.1