Re: [PATCH] mm/damon/core: reject empty access pattern intervals on scheme commit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/damon: damos_validate_scheme() breaks DAMON sysfs.py selftest
--

commit f1effb4d8401c0508f7cb04b14b9b55d0d6abd17
Author: Xiaolei Wang (Kaihong) <xiaoleiwangxiaolei@gmail.com>

mm/damon/core: reject empty access pattern intervals on scheme commit

This commit introduces damos_validate_scheme() and calls it at the start of
damos_commit() to reject DAMOS schemes with reversed access pattern bounds
(min > max) or non-monotonic watermark thresholds with -EINVAL.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index d36823f4d0928..9b2b2a63a2576 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1561,10 +1561,29 @@ static struct damos *damon_nth_scheme(int n, struct damon_ctx *ctx)
>  	return NULL;
>  }
>  
> +static int damos_validate_scheme(struct damos *s)
> +{
> +	struct damos_access_pattern *p = &s->pattern;
> +	struct damos_watermarks *wmarks = &s->wmarks;
> +
> +	if (p->min_sz_region > p->max_sz_region ||
> +	    p->min_nr_accesses > p->max_nr_accesses ||
> +	    p->min_age_region > p->max_age_region)
> +		return -EINVAL;

[Severity: Medium]
Does this validation break the existing DAMON sysfs kselftest in
tools/testing/selftests/damon/sysfs.py?

In tools/testing/selftests/damon/sysfs.py, main() constructs a non-default
scheme for testing online parameter commitment with min_sz_region = 4096 and
max_sz_region = 1024:

tools/testing/selftests/damon/sysfs.py:main()
            schemes=[_damon_sysfs.Damos(
                action='pageout',
                access_pattern=_damon_sysfs.DamosAccessPattern(
                    size=[4096, 2**10],
                    nr_accesses=[3, 317],
                    age=[5,71]),

When main() subsequently calls kdamonds.kdamonds[0].commit() and
assert_ctxs_committed(kdamonds), writing commit to the kdamond state sysfs
file invokes:

state_store()
  -> damon_sysfs_handle_cmd()
    -> damon_sysfs_commit_input()
      -> damon_commit_ctx()
        -> __damon_commit_ctx()
          -> damon_commit_schemes()
            -> damos_commit()
              -> damos_validate_scheme()

Because p->min_sz_region (4096) > p->max_sz_region (1024),
damos_validate_scheme() returns -EINVAL, causing kd.commit() in
assert_ctxs_committed() to fail and exit with status 1:

tools/testing/selftests/damon/sysfs.py:assert_ctxs_committed()
        if kd in kdamonds_paused_for_dump:
            err = kd.commit()
            if err is not None:
                print('pause fail (%s)' % err)
                kdamonds.stop()
                exit(1)

Should tools/testing/selftests/damon/sysfs.py also be updated to use a
non-empty size range (for example, size=[4096, 2**20]) so the selftest
continues to pass?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103105418.362459-1-xiaoleiwangxiaolei@gmail.com?part=1