Re: mm/DAMON: Profiling enhancements for DAMON

From: Yu Zhao <yuzhao@google.com>
To: "Prasad, Aravinda" <aravinda.prasad@intel.com>
Cc: "damon@lists.linux.dev" <damon@lists.linux.dev>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	 "sj@kernel.org" <sj@kernel.org>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"s2322819@ed.ac.uk" <s2322819@ed.ac.uk>,
	 "Kumar, Sandeep4" <sandeep4.kumar@intel.com>,
	"Huang, Ying" <ying.huang@intel.com>,
	 "Hansen, Dave" <dave.hansen@intel.com>,
	"Williams, Dan J" <dan.j.williams@intel.com>,
	 "Subramoney, Sreenivas" <sreenivas.subramoney@intel.com>,
	 "Kervinen, Antti" <antti.kervinen@intel.com>,
	 "Kanevskiy, Alexander" <alexander.kanevskiy@intel.com>,
	Alan Nair <alan.nair@intel.com>,  Juergen Gross <jgross@suse.com>,
	Ryan Roberts <ryan.roberts@arm.com>
Subject: Re: mm/DAMON: Profiling enhancements for DAMON
Date: Fri, 15 Dec 2023 22:41:42 -0700	[thread overview]
Message-ID: <CAOUHufbDzy5dMcLR9ex25VdB_QBmSrW_We-2+KftZVYKNn4s9g@mail.gmail.com> (raw)
In-Reply-To: <MW5PR11MB5907BA0C9546C069925C9955F293A@MW5PR11MB5907.namprd11.prod.outlook.com>

On Fri, Dec 15, 2023 at 3:08 AM Prasad, Aravinda
<aravinda.prasad@intel.com> wrote:
>
> > On Fri, Dec 15, 2023 at 12:42 AM Aravinda Prasad
> > <aravinda.prasad@intel.com> wrote:
> > ...
> >
> > > This patch proposes profiling different levels of the application’s
> > > page table tree to detect whether a region is accessed or not. This
> > > patch is based on the observation that, when the accessed bit for a
> > > page is set, the accessed bits at the higher levels of the page table
> > > tree (PMD/PUD/PGD) corresponding to the path of the page table walk
> > > are also set. Hence, it is efficient to  check the accessed bits at
> > > the higher levels of the page table tree to detect whether a region is
> > > accessed or not.
> >
> > This patch can crash on Xen. See commit 4aaf269c768d("mm: introduce
> > arch_has_hw_nonleaf_pmd_young()")
>
> Will fix as suggested in the commit.
>
> >
> > MGLRU already does this in the correct way. See mm/vmscan.c.
>
> I don't see access bits at PUD or PGD checked for 4K page size. Can you
> point me to the code where access bits are checked at PUD and PGD level?

There isn't any, because *the system* bottlenecks at the PTE level and
at moving memory between tiers. Optimizing at the PUD/PGD levels has
insignificant ROI for the system.

And food for thought:
1. Can a PUD/PGD cover memory from different tiers?
2. Can the A-bit in non-leaf entries work for EPT?

> > This patch also can cause USER DATA CORRUPTION. See commit
> > c11d34fa139e ("mm/damon/ops-common: atomically test and clear young
> > on ptes and pmds").
>
> Ok. Will atomically test and set the access bits.
>
> >
> > The quality of your patch makes me very much doubt the quality of your
> > paper, especially your results on Google's kstaled and MGLRU in table 6.2.
>
> The results are very much reproducible. We have not used kstaled/MGLRU for
> the data in Figure 3, but we linearly scan pages similar to kstaled by implementing
> a kernel thread for scanning.

You have not used MGLRU, and yet your results are very much reproducible.

> Our argument for kstaled/MGLRU is that, scanning individual pages at 4K
> granularity may not be efficient for large footprint applications.

Your argument for MGLRU is based on a wrong assumption, as I have
already pointed out.

> Instead,
> access bits at the higher level of the page table tree can be used. In the
> paper we have demonstrated this with DAMON but the concept can be
> applied to kstaled/MGLRU as well.

You got it backward: MGLRU introduced the concept; you fabricated a
comparison table.