lkml.org 
[lkml]   [2023]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: mm/DAMON: Profiling enhancements for DAMON
Date


> -----Original Message-----
> From: Hansen, Dave <dave.hansen@intel.com>
> Sent: Friday, December 15, 2023 11:40 PM
> To: Prasad, Aravinda <aravinda.prasad@intel.com>; damon@lists.linux.dev; linux-
> mm@kvack.org; sj@kernel.org; linux-kernel@vger.kernel.org
> Cc: s2322819@ed.ac.uk; Kumar, Sandeep4 <sandeep4.kumar@intel.com>;
> Huang, Ying <ying.huang@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>
> 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.

\
 
 \ /
  Last update: 2023-12-18 12:55    [W:0.127 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site