On Thu, 29 Dec 2022 21:06:12 +0000 Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> On Wed, Dec 28, 2022 at 07:34:11PM +0800, Kefeng Wang wrote:
> > - if (pmd_young(*pmd) || !page_is_idle(page) ||
> > + if (pmd_young(*pmd) || !folio_test_idle(folio) ||
> > mmu_notifier_test_young(walk->mm,
> > addr)) {
> > *priv->page_sz = HPAGE_PMD_SIZE;
>
> hmm ...
>
> > pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl);
> > if (!pte_present(*pte))
> > goto out;
> > - page = damon_get_page(pte_pfn(*pte));
> > - if (!page)
> > + folio = damon_get_folio(pte_pfn(*pte));
> > + if (!folio)
> > goto out;
> > - if (pte_young(*pte) || !page_is_idle(page) ||
> > + if (pte_young(*pte) || !folio_test_idle(folio) ||
> > mmu_notifier_test_young(walk->mm, addr)) {
> > *priv->page_sz = PAGE_SIZE;
>
> hmm ...
>
> So why aren't we doing '*priv->page_sz = folio_size(folio)'? What does
> DAMON want to do when encountering folios that are neither PAGE_SIZE
> nor HPAGE_PMD_SIZE?
Good point. We use the field to know if next address to check access is in
same folio and therefore if we could reuse the last access check result. So it
would be better to use 'folio_size(folio)' here. The field name would also
better to be 'folio_sz'. I will make the change, unless others do.
Thanks,
SJ
