Re: [PATCH v4 2/6] mm/damon/paddr: use damon_get_folio_in_region to obtain folio

[SeongJae Park <sj@kernel.org>]

On Mon,  3 Feb 2025 22:55:29 +0000 Usama Arif <usamaarif642@gmail.com> wrote:

> This is introduced for larger folios. If a large folio has subpages
> present in multiple regions, it will be considered multiple times.
> This can be when checking access

I think this behavior makes sense.  If a 2 MiB address range is known to be
accessed or not, treating DAMON regions in the address range as all accessed or
not aligns with the DAMON's region definition and makes sense in general, to
me.  Note that DAMON's adaptive regions adjustment mechanism will make the
DAMON regions in a large folio to be merged in near future.

> or applying DAMOS schemes. For e.g.
> in pa_stat, folios split across N regions will be counted N times,
> giving inaccurate results.

Nice catch!  I agree this could be a problem.

> Hence, only consider a page for access check/DAMOS scheme only if the head
> page is part of that region as well.

For access checking, this seems wrong to me.  This change will unnecessarily
mark some DAMON regions as not accessed.

Even for DAMOS, this seems not very optimum.

1. DAMOS will unnecessarily repeat PAGE_SIZE skippings on second and later
   DAMON regions of a same large folio.
2. If only a small part of the large folio belongs to the first DAMON region,
   and the DAMON region is not eligible to the scheme, while the second region
   is, the scheme action will not applied to the second region.

Also, I think this problem can happen on vaddr case.  Hence, making the change
on core layer makes sense to me.

That is, let damon_ops->apply_scheme() inform the caller (damos_apply_scheme())
how much bytes of the next region should be ignored at applying the action.
Due to the complicated implementation of DAMOS core logic, this might be not
very simple.

I think the additional complexity might outweigh the benefit for following
reasons.  First, adaptive regions adjustment mechanism will make DAMON regions
in same large folios to be merged soon.  So the problem will be not significant
in common case.  Second, any threads could collapse any parts of memory into
large folios while DAMOS is traversing DAMON regions.  So this problem cannot
perfectly be solved unless we make DAMOS' DAMON regions traversal exclusive
with any large folios collapsing.  Which would add more complexity.

And given DAMON's best-effort nature, keeping the issue until we get a simple
and effective solution is ok to me, unless a real significant issue from this
is confirmed.

Usama, what do you think?


Thanks,
SJ

[...]