Skip to contentSkip to conversation

mm/damon/core: disallow non-power of two min_region_sz on damon_start()

PATCH Applied SeongJae Park 1 participant 2 messages Apr 11

Subsystems (3)
Stable branch Memory management DAMON
lore
Share & tools
lore

SeongJae Park

author maintainer
#0 lore raw

PATCH 1/1 mm/damon/core: disallow non-power of two min_region_sz on damon_start()

Commit d8f867fa0825 ("mm/damon: add damon_ctx->min_sz_region") introduced a bug that allows unaligned DAMON region address ranges. Commit c80f46ac228b ("mm/damon/core: disallow non-power of two min_region_sz") fixed it, but only for damon_commit_ctx() use case. Still, DAMON sysfs interface can emit non-power of two min_region_sz via damon_start(). Fix the path by adding the is_power_of_2() check on damon_start().

The issue was discovered by sashiko [1].

[1] https://lore.kernel.org/[email protected]

Fixes d8f867fa0825 ("mm/damon: add damon_ctx->min_sz_region")
Cc <[email protected]> # 6.18.x
Signed-off-by SeongJae Park <[email protected]>
---
 mm/damon/core.c | 5 +++++
 1 file changed, 5 insertions(+)
mm/damon/core.c +5 -0
index 3703f62a876b3..c107d74c77e74 100644
@@ -1368,6 +1368,11 @@ int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
 	int i;
 	int err = 0;
 
+	for (i = 0; i < nr_ctxs; i++) {
+		if (!is_power_of_2(ctxs[i]->min_region_sz))
+			return -EINVAL;
+	}
+
 	mutex_lock(&damon_lock);
 	if ((exclusive && nr_running_ctxs) ||
 			(!exclusive && running_exclusive_ctxs)) {

base-commit: 872b6168e577ea326324255fa8b6716a89124680

Signature · 1 line
2.47.3
author maintainer
#1 lore raw

Re: (sashiko review) [PATCH] mm/damon/core: disallow non-power of two min_region_sz on damon_start()

TL; DR: sashiko found no blocker 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.

Quoted 42 lines
# review url: https://sashiko.dev/#/patchset/[email protected]
# start of sashiko.dev inline review
commit a125056db470cab021e84ad481a0f51ddfc426c2
Author: SeongJae Park <[email protected]>

mm/damon/core: disallow non-power of two min_region_sz on damon_start()

This commit fixes an issue that allows unaligned DAMON region address ranges. It adds a check for power-of-two min_region_sz directly in damon_start() to catch invalid configurations from the sysfs interface.

Link https://lore.kernel.org/[email protected] open here

mm/damon/core.c int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive) · line 1375

Quoted 13 lines
mm/damon/core.c +5 -0
index 3703f62a876b3..c107d74c77e74 100644
@@ -1368,6 +1368,11 @@ int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
 	int i;
 	int err = 0;
 
+	for (i = 0; i < nr_ctxs; i++) {
+		if (!is_power_of_2(ctxs[i]->min_region_sz))
+			return -EINVAL;
+	}
+

This isn't a regression introduced by this commit, but modifying damon_start() brings up a question about partial failure handling when starting multiple contexts.

If __damon_start() succeeds for the first context but fails for a subsequent one (for example if kthread_run() fails), the initialization loop breaks and returns an error:

	for (i = 0; i < nr_ctxs; i++) {
		err = __damon_start(ctxs[i]);
		if (err)
			break;
		nr_running_ctxs++;
	}
Quoted 8 lines

Does this leave the previously successful kdamond threads running without stopping them?

Because nr_running_ctxs is incremented for the successful starts and running_exclusive_ctxs is set to true, it appears the internal state would prevent starting new contexts.

Would this result in all future calls to damon_start() returning -EBUSY?

Yes, this is not ideal. But as Sashiko also says it is no blocker of this patch. I will revisit this later.

Quoted 4 lines

# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/[email protected]

Thanks,
SJ

# hkml [1] generated a draft of this mail.  You can regenerate
# this using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             [email protected]
#
# [1] https://github.com/sjp38/hackermail