Re: [PATCH REPOST 5/5] mm/damon/vaddr: support pgidle_unset probe filter type

[KunWu Chan]

On Fri, Sep 11, 2026 at 9:32 PM SJ Park <sj@xxxxxxxxxx> wrote:
>
> Hi Kunwu,
>
> On Fri, 11 Sep 2026 18:04:26 +0800 Kunwu Chan <kunwu.chan@xxxxxxxxx> wrote:
>
> > On Wed,  9 Sep 2026 07:04:07 -0700 SJ Park <sj@xxxxxxxxxx> wrote:
> >
> > > DAMON virtual address space operation set (vaddr) does not support
> > > pgidle_unset probe filter type.  Add the support.
> > >
> > > Signed-off-by: SJ Park <sj@xxxxxxxxxx>
> > > ---
> > >  mm/damon/vaddr.c | 55 ++++++++++++++++++++++++++++++++++++++++++------
> > >  1 file changed, 48 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > > index 45239f05e113b..9a38dc89a156e 100644
> > > --- a/mm/damon/vaddr.c
> > > +++ b/mm/damon/vaddr.c
> > > @@ -522,13 +522,49 @@ static void damon_va_prep_probes(struct damon_ctx *ctx, bool set_samples)
> > >     }
> > >  }
> > >
> > > -static bool damon_va_filter_pass(struct folio *folio, struct damon_probe *p)
> > > +static bool damon_va_young_addr(struct folio *folio, pte_t *pte, pmd_t *pmd,
> > > +           struct mm_struct *mm, unsigned long addr)
> > > +{
> > > +   bool young = false;
> > > +
> > > +   if (pte)
> > > +           young = pte_young(*pte);
> > > +   else if (pmd)
> > > +           young = pmd_young(*pmd);
> >
> > Hi SJ,
> > I see that all callers already check pte_present() / pmd_present()
> > before calling this helper, so I don't see a correctness issue here.
> >
> > However, the other DAMON young/accessed helpers, such as
> > damon_pte_young(), damon_hugetlb_young(), and the PMD path in
> > damon_folio_young_one(), keep the present check together with the
> > young check.
> >
> > Would it make sense to keep the same check here for consistency?
>
> In my humble opinion, it is already consistent as long as we didn't miss
> present check in callers.  If you concern a case that we call
> damon_va_young_addr() without present check in future, how about adding a
> comment noting that on damon_va_young_addr() as a followup work?
>

Thanks, SJ. That makes sense to me.

The current callers do provide the required ‘present’ check, so I agree
that documenting the assumption in a follow-up comment should be
sufficient.

Reviewed-by: Kunwu Chan <kunwu.chan@xxxxxxxxx>

Thanks,
Kunwu

> Also, thank you for your reviews to this series!
>
>
> Thanks,
> SJ
>
> [...]