Messages in this thread | | | From | "Prasad, Aravinda" <> | Subject | RE: mm/DAMON: Profiling enhancements for DAMON | Date | Mon, 18 Dec 2023 11:41:33 +0000 |
| |
> -----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.
| |