[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <MW5PR11MB59074127DBF2AC23AEFF4EF2F290A@MW5PR11MB5907.namprd11.prod.outlook.com>
Date: Mon, 18 Dec 2023 11:41:33 +0000
From: "Prasad, Aravinda" <aravinda.prasad@...el.com>
To: "Hansen, Dave" <dave.hansen@...el.com>, "damon@...ts.linux.dev"
<damon@...ts.linux.dev>, "linux-mm@...ck.org" <linux-mm@...ck.org>,
"sj@...nel.org" <sj@...nel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
CC: "s2322819@...ac.uk" <s2322819@...ac.uk>, "Kumar, Sandeep4"
<sandeep4.kumar@...el.com>, "Huang, Ying" <ying.huang@...el.com>, "Williams,
Dan J" <dan.j.williams@...el.com>, "Subramoney, Sreenivas"
<sreenivas.subramoney@...el.com>, "Kervinen, Antti"
<antti.kervinen@...el.com>, "Kanevskiy, Alexander"
<alexander.kanevskiy@...el.com>, Alan Nair <alan.nair@...el.com>
Subject: RE: mm/DAMON: Profiling enhancements for DAMON
> -----Original Message-----
> From: Hansen, Dave <dave.hansen@...el.com>
> Sent: Friday, December 15, 2023 11:40 PM
> To: Prasad, Aravinda <aravinda.prasad@...el.com>; damon@...ts.linux.dev; linux-
> mm@...ck.org; sj@...nel.org; linux-kernel@...r.kernel.org
> Cc: s2322819@...ac.uk; Kumar, Sandeep4 <sandeep4.kumar@...el.com>;
> Huang, Ying <ying.huang@...el.com>; Williams, Dan J
> <dan.j.williams@...el.com>; Subramoney, Sreenivas
> <sreenivas.subramoney@...el.com>; Kervinen, Antti <antti.kervinen@...el.com>;
> Kanevskiy, Alexander <alexander.kanevskiy@...el.com>; Alan Nair
> <alan.nair@...el.com>
> Subject: Re: mm/DAMON: Profiling enhancements for DAMON
>
> On 12/14/23 23:46, Aravinda Prasad wrote:
> > +static int damon_young_pmd(pmd_t *pmd, unsigned long addr,
> > + unsigned long next, struct mm_walk *walk) {
> > + spinlock_t *ptl;
> > + struct damon_young_walk_private *priv = walk->private;
> > +
> > + if (!pmd_present(*pmd) || pmd_none(*pmd))
> > + goto out;
> > +
> > + ptl = pmd_lock(walk->mm, pmd);
> > + if (pmd_young(*pmd) || mmu_notifier_test_young(walk->mm, addr))
> > + priv->young = true;
> > +
> > + *priv->folio_sz = (1UL << PMD_SHIFT);
> > + spin_unlock(ptl);
> > +out:
> > + return 0;
> > +}
>
> There are a number of paired p*_present() and p_*none() checks in this patch.
> What is their function (especially pmd_none())? For instance,
> damon_young_pmd() gets called via the pagewalk infrastructure:
Hmm.. I think, they can be removed. Looks like pagewalk infrastructure should take care.
But will double check.
>
> > static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
> > struct mm_walk *walk) {
> ...
> > next = pmd_addr_end(addr, end);
> > if (pmd_none(*pmd)) {
> ...
> > continue;
> > }
> ...
> > if (ops->pmd_entry)
> > err = ops->pmd_entry(pmd, addr, next, walk);
>
> I'd suggest taking a closer look at the code you are replacing and other mm_walk
> users to make sure the handlers are doing appropriate checks.
Sure.
Powered by blists - more mailing lists