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:
> 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.
