Re: [RFC PATCH 3/6] selftests/damon/_damon_sysfs: implement DamonProbes
SJ Park <[email protected]> Wed, 29 Jul 2026 17:06:23 -0700
| Newsgroups | dev.linux.lists.damon |
|---|---|
| Message-ID | <[email protected]> |
On Wed, 29 Jul 2026 15:04:39 +0000 [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] The error result of `write_file()` for setting the probe weight is silently discarded. > -- > > --- Patch [3]: [RFC PATCH 3/6] selftests/damon/_damon_sysfs: implement DamonProbes --- > commit 12ef2c079bfc3ec5e9f25fc085e18c6bca931488 > Author: SJ Park <[email protected]> > > selftests/damon/_damon_sysfs: implement DamonProbes > > Extend _damon_sysfs.py to support staging and committing DAMON probes. > It will be used for setting DAMON probes via sysfs changes for testing > purposes. > > > diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py > > --- a/tools/testing/selftests/damon/_damon_sysfs.py > > +++ b/tools/testing/selftests/damon/_damon_sysfs.py > > [ ... ] > > > +class DamonProbe: > > + weight = None > > + filters = None > > + probes = None > > + idx = None > > + > > + def __init__(self, weight=0, filters=None): > > + self.weight = weight > > + if filters is None: > > + filters = DamonFilters() > > + self.filters = filters > > + self.filters.probe = self > > + > > + def sysfs_dir(self): > > + return os.path.join(self.probes.sysfs_dir(), '%d' % self.idx) > > + > > + def stage(self): > > + err = write_file( > > + os.path.join(self.sysfs_dir(), 'weight'), '%d' % self.weight) > > + return self.filters.stage() > > [Severity: Medium] > Is err intentionally ignored here? If write_file() fails to write the weight > to sysfs, it returns an error string. Returning self.filters.stage() directly > without checking err will swallow the error and might cause tests to proceed > with incorrect assumptions about the probe configuration. > > Should this check err before returning, similar to how it is handled in other > stage() methods? Good catch, I will fix. ' > > -- > Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3 > Thanks, SJ