Re: [PATCH v2 1/3] docs/mm/damon/design: accurate semantics of nr_snapshots

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The commit updates documentation to match buggy code behavior rather than fixing the underlying logical flaws in the `nr_snapshots` counter. `nr_snapshots` incorrectly increments per target instead of per snapshot, and skips entirely when quotas are full.
--

--- Patch [1]: [PATCH v2 1/3] docs/mm/damon/design: accurate semantics of nr_snapshots ---
commit 11b4a9c760b881c063ff5e1eec0248b087d6a31f
Author: Liew Rui Yan <[email protected]>

docs/mm/damon/design: accurate semantics of nr_snapshots

This commit updates the DAMON design documentation and header comments
to align the description of nr_snapshots with the current behavior
implemented in the code.

> diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> index aed6cb1cf4831..1739aeec6eb95 100644
> --- a/Documentation/mm/damon/design.rst
> +++ b/Documentation/mm/damon/design.rst
> @@ -846,8 +846,8 @@ scheme's execution.
[ ... ]
>  - ``qt_exceeds``: Total number of times the quota of the scheme has exceeded.
> -- ``nr_snapshots``: Total number of DAMON snapshots that the scheme is tried to
> -  be applied.
> +- ``nr_snapshots``: Total number of DAMON snapshots that the scheme is
> +  completely tried to be applied.
>  - ``max_nr_snapshots``: Upper limit of ``nr_snapshots``.

[Severity: High]
This is a pre-existing issue, but does updating the documentation to match
the code behavior paper over an underlying logical flaw in the nr_snapshots
counter?

Looking at damon_do_apply_schemes() in mm/damon/core.c, the counter updates
like this:

mm/damon/core.c:damon_do_apply_schemes() {
    ...
        /* Check the quota */
        if (damos_quota_is_full(quota, c->min_region_sz))
            continue;
    ...
        if (damon_is_last_region(r, t))
            s->stat.nr_snapshots++;
    ...
}

Because this increments at the last region of a target, if multiple targets
are present, does the counter incorrectly increment per target instead of
per snapshot?

Additionally, if a restrictive quota fills up before the last region of a
target is processed, the early continue skips the counter increment entirely.
This could mean a scheme configured with a max_nr_snapshots limit might
never reach its limit.

Should the underlying code be fixed to increment once per snapshot regardless
of quotas and targets, rather than updating the documentation to reflect this
behavior?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1