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

[SeongJae Park]

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@xxxxxxxxxx
> # start of sashiko.dev inline review
> commit 47cf42ea4e5f609d65881ae5f8f8491bb884a8bf
> Author: SeongJae Park <sj@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx


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@xxxxxxxxxx
#
# [1] https://github.com/sjp38/hackermail