Re: [PATCH v2] mm/damon/core: fix commit_ops_filters by using correct nth function

Hi SeongJae

On Sun, Aug 10, 2025 at 2:48 AM SeongJae Park sj@kernel.org wrote:

On Sat, 9 Aug 2025 22:07:56 +0900 Sang-Heon Jeon ekffu200098@gmail.com wrote:

...

damos_commit_ops_filters() incorrectly uses damos_nth_filter() which iterates core_filters. As a result, performing a commit unintentionally corrupts ops_filters.

Add damos_nth_ops_filter() which iterates ops_filters. Use this function to fix issues caused by wrong iteration.

Also, add test to verify that modification is right way.

As I mentioned on v1 thread, please drop the test part, for ease of backporting.

As I mentioned on the v1 thread as well, I'll separate them back.

...

Fixes: 3607cc590f18 ("mm/damon/core: support committing ops_filters") # 6.15.x Cc: stable@vger.kernel.org Signed-off-by: Sang-Heon Jeon ekffu200098@gmail.com

---

Changes from v1 [1]:

1. Fix code and commit message style.
2. Merge patch set into one patch.
3. Add fixes and cc section for backporting.

[1] https://lore.kernel.org/damon/20250808195518.563053-1-ekffu200098@gmail.com/

---

I tried to fix your all comments, but maybe i miss something. Then please let me know; I'll fix it as soon as possible.

---

mm/damon/core.c | 14 +++- tools/testing/selftests/damon/Makefile | 1 + .../damon/sysfs_no_op_commit_break.py | 72 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100755 tools/testing/selftests/damon/sysfs_no_op_commit_break.py

diff --git a/mm/damon/core.c b/mm/damon/core.c index 883d791a10e5..19c8f01fc81a 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -862,6 +862,18 @@ static struct damos_filter *damos_nth_filter(int n, struct damos *s) return NULL; }

+static struct damos_filter *damos_nth_ops_filter(int n, struct damos *s) +{

• struct damos_filter *filter;
  

• int i = 0;
  

• damos_for_each_ops_filter(filter, s) {
  

•         if (i++ == n)
  

•                 return filter;
  

• }
  

• return NULL;
  

+}

static void damos_commit_filter_arg( struct damos_filter *dst, struct damos_filter *src) { @@ -925,7 +937,7 @@ static int damos_commit_ops_filters(struct damos *dst, struct damos *src) int i = 0, j = 0;

  damos_for_each_ops_filter_safe(dst_filter, next, dst) {

•         src_filter = damos_nth_filter(i++, src);
  

•         src_filter = damos_nth_ops_filter(i++, src);
          if (src_filter)
                  damos_commit_filter(dst_filter, src_filter);
          else
  

diff --git a/tools/testing/selftests/damon/Makefile b/tools/testing/selftests/damon/Makefile index 5b230deb19e8..44a4a819df55 100644 --- a/tools/testing/selftests/damon/Makefile +++ b/tools/testing/selftests/damon/Makefile @@ -17,6 +17,7 @@ TEST_PROGS += reclaim.sh lru_sort.sh TEST_PROGS += sysfs_update_removed_scheme_dir.sh TEST_PROGS += sysfs_update_schemes_tried_regions_hang.py TEST_PROGS += sysfs_memcg_path_leak.sh +TEST_PROGS += sysfs_no_op_commit_break.py

EXTRA_CLEAN = __pycache__

diff --git a/tools/testing/selftests/damon/sysfs_no_op_commit_break.py b/tools/testing/selftests/damon/sysfs_no_op_commit_break.py new file mode 100755 index 000000000000..fbefb1c83045 --- /dev/null +++ b/tools/testing/selftests/damon/sysfs_no_op_commit_break.py @@ -0,0 +1,72 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0

+import json +import os +import subprocess +import sys

+import _damon_sysfs

+def dump_damon_status_dict(pid):

• try:

•    subprocess.check_output(['which', 'drgn'], stderr=subprocess.DEVNULL)
  

• except:

•    return None, 'drgn not found'
  

• file_dir = os.path.dirname(os.path.abspath(__file__))
• dump_script = os.path.join(file_dir, 'drgn_dump_damon_status.py')
• rc = subprocess.call(['drgn', dump_script, pid, 'damon_dump_output'],

•    stderr=subprocess.DEVNULL)
  

• if rc != 0:

•    return None, f'drgn fail: return code({rc})'
  

• try:

•    with open('damon_dump_output', 'r') as f:
  

•        return json.load(f), None
  

• except Exception as e:

•    return None, 'json.load fail (%s)' % e
  

+def main():

• kdamonds = _damon_sysfs.Kdamonds(

•    [_damon_sysfs.Kdamond(
  

•        contexts=[_damon_sysfs.DamonCtx(
  

•            schemes=[_damon_sysfs.Damos(
  

•                ops_filters=[
  

•                    _damon_sysfs.DamosFilter(
  

•                        type_='anon',
  

•                        matching=True,
  

•                        allow=True,
  

•                    )
  

•                ]
  

•            )],
  

•        )])]
  

• )
• err = kdamonds.start()
• if err is not None:

•    print('kdamond start failed: %s' % err)
  

•    exit(1)
  

• before_commit_status, err = \

•    dump_damon_status_dict(kdamonds.kdamonds[0].pid)
  

• if err is not None:

•    print(err)
  

Adding more context would be nice, e.g.,

print('before-commit status dump failed: %s' % err)

Sure, I'll add that to v3 patch.

...

•    exit(1)
  

• kdamonds.kdamonds[0].commit()
• after_commit_status, err = \

•    dump_damon_status_dict(kdamonds.kdamonds[0].pid)
  

• if err is not None:

•    print(err)
  

Adding more context would be nice, e.g.,

print('after-commit status dump failed: %s' % err)

ditto.

...

•    exit(1)
  

• if before_commit_status != after_commit_status:

•    print(f'before: {json.dump(before_commit_status, indent=2)}')
  

•    print(f'after: {json.dump(after_commit_status, indent=2)}')
  

•    exit(1)
  

• kdamonds.stop()

+if __name__ == '__main__':

• main()

-- 2.43.0

Other than above two trivial comments, nothing stands out to me.

Sounds great!

Thanks, SJ

Best Regards. Sang-Heon Jeon.