* [PATCH v2] mm/damon/core: fix commit_ops_filters by using correct nth function
@ 2025-08-09 13:07 Sang-Heon Jeon
2025-08-09 17:48 ` SeongJae Park
0 siblings, 1 reply; 3+ messages in thread
From: Sang-Heon Jeon @ 2025-08-09 13:07 UTC (permalink / raw)
To: sj, honggyu.kim; +Cc: damon, linux-mm, Sang-Heon Jeon, stable
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.
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)
+ 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)
+ 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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] mm/damon/core: fix commit_ops_filters by using correct nth function
2025-08-09 13:07 [PATCH v2] mm/damon/core: fix commit_ops_filters by using correct nth function Sang-Heon Jeon
@ 2025-08-09 17:48 ` SeongJae Park
2025-08-10 5:50 ` Sang-Heon Jeon
0 siblings, 1 reply; 3+ messages in thread
From: SeongJae Park @ 2025-08-09 17:48 UTC (permalink / raw)
To: Sang-Heon Jeon; +Cc: SeongJae Park, honggyu.kim, damon, linux-mm, stable
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.
>
> 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)
> + 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)
> + 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.
Thanks,
SJ
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] mm/damon/core: fix commit_ops_filters by using correct nth function
2025-08-09 17:48 ` SeongJae Park
@ 2025-08-10 5:50 ` Sang-Heon Jeon
0 siblings, 0 replies; 3+ messages in thread
From: Sang-Heon Jeon @ 2025-08-10 5:50 UTC (permalink / raw)
To: SeongJae Park; +Cc: honggyu.kim, damon, linux-mm, stable
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-10 5:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-09 13:07 [PATCH v2] mm/damon/core: fix commit_ops_filters by using correct nth function Sang-Heon Jeon
2025-08-09 17:48 ` SeongJae Park
2025-08-10 5:50 ` Sang-Heon Jeon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).