Re: (sashiko review) [RFC PATCH v2 7/9] selftets/damon/_damon_sysfs: support failed region quota charge ratio

SeongJae Park
2026-04-05T19:41:07Z

TL; DR: sashiko found a typo and a false positive issue. I will fix the typo
in the next revision of this patch.

Forwarding full sashiko review in a reply format with my
inline comments below, for sharing details of my view and
doing followup discussions via mails if needed.

> # review url: https://sashiko.dev/#/patchset/20260405151232.102690-8-sj@kernel.org
> # start of sashiko.dev inline review
> commit 47cf42ea4e5f609d65881ae5f8f8491bb884a8bf
> Author: SeongJae Park <sj@kernel.org>
>
> selftets/damon/_damon_sysfs: support failed region quota charge ratio
>
> This commit extends the _damon_sysfs.py selftests utility to support the
> new DAMOS action failed regions quota charge ratio feature.
>
> > selftets/damon/_damon_sysfs: support failed region quota charge ratio
>
> This isn't a bug, but there is a typo (selftets) in the subject line.

Good eye, I will fix it in the next revision.

>
> > diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py
> > index 2b4df655d9fd0..0f13512fa5e6d 100644
> > --- a/tools/testing/selftests/damon/_damon_sysfs.py
> > +++ b/tools/testing/selftests/damon/_damon_sysfs.py
> [ ... ]
> > @@ -132,14 +132,17 @@ class DamosQuota:
> [ ... ]
> > def __init__(self, sz=0, ms=0, goals=None, goal_tuner='consist',
> > - reset_interval_ms=0, weight_sz_permil=0,
> > - weight_nr_accesses_permil=0, weight_age_permil=0):
> > + reset_interval_ms=0, fail_charge_num=0, fail_charge_denom=0,
> > + weight_sz_permil=0, weight_nr_accesses_permil=0,
> > + weight_age_permil=0):
>
> Will inserting fail_charge_num and fail_charge_denom before the weight
> parameters cause issues for any existing code that passes arguments
> positionally?
>
> Any scripts or in-flight patches instantiating DamosQuota using positional
> arguments for the weight parameters might silently assign those values to
> the new charge ratio variables instead.
>
> Could these optional parameters be added to the end of the argument list to
> avoid altering the existing positional signature?

No. There is no existing code that calls DAmosQuota() with positional
arguments. Hence no change to this patch is needed.

>
>
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260405151232.102690-8-sj@kernel.org

Thanks,
SJ

# hkml [1] generated a draft of this mail. You can regenerate
# this using below command:
#
# hkml patch sashiko_dev --for_forwarding \
# 20260405151232.102690-8-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail