[PATCH v2 0/3] cgroup/dmem: allow atomic irrestrictive writes to dmem.max

[PATCH v2 0/3] cgroup/dmem: allow atomic irrestrictive writes to dmem.max

[Thadeu Lima de Souza Cascardo]

When writing to dmem.max, it was noticed that some writes did not take
effect, even though the write was successful.

It turns out that page_counter_set_max checks if the new max value is above
the current usage and returns -EBUSY in case it is, which was being ignored
by dmemcg_limit_write.

It was also noticed that when setting limits for multiple regions in a
single write, while setting one region's limit may fail, others might have
succeeded before. Tejun Heo brought up that this breaks atomicity.

Maarten Lankhorst and Michal Koutný have brought up that instead of
failing, setting max below the current usage should behave like memcg and
start eviction until usage goes below the new max.

However, since there is no current mechanism to evict a given region, they
suggest that setting the new max will at least prevent further allocations.

v1 kept the multiple region support when writing to limit files, while
returning -EBUSY as soon as setting a region's max was above the usage.

This version (v2) only allows setting a single region limit per write,
while allowing any new max to be set.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
Changes in v2:
- Remove support for setting multiple regions' limits.
- Allow any new max limit to be set.
- Link to v1: https://lore.kernel.org/r/20260318-dmem_max_ebusy-v1-1-b7e461157b29@igalia.com

---
Thadeu Lima de Souza Cascardo (3):
      cgroup/dmem: remove region parameter from dmemcg_parse_limit
      cgroup/dmem: accept a single region when writing to attributes
      cgroup/dmem: allow max to be set below current usage

 kernel/cgroup/dmem.c | 74 +++++++++++++++++++++++-----------------------------
 1 file changed, 32 insertions(+), 42 deletions(-)
---
base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
change-id: 20260318-dmem_max_ebusy-bfd33564f2c3

Best regards,
-- 
Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

[PATCH v2 1/3] cgroup/dmem: remove region parameter from dmemcg_parse_limit

[Thadeu Lima de Souza Cascardo]

dmemcg_parse_limit does not use the region parameter. Remove it.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
 kernel/cgroup/dmem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
index 9d95824dc6fa09422274422313b63c25986596de..1ab1fb47f2711ecc60dd13e611a8a4920b48f3e9 100644
--- a/kernel/cgroup/dmem.c
+++ b/kernel/cgroup/dmem.c
@@ -707,8 +707,7 @@ static int dmem_cgroup_region_capacity_show(struct seq_file *sf, void *v)
 	return 0;
 }
 
-static int dmemcg_parse_limit(char *options, struct dmem_cgroup_region *region,
-			      u64 *new_limit)
+static int dmemcg_parse_limit(char *options, u64 *new_limit)
 {
 	char *end;
 
@@ -762,7 +761,7 @@ static ssize_t dmemcg_limit_write(struct kernfs_open_file *of,
 		if (!region)
 			return -EINVAL;
 
-		err = dmemcg_parse_limit(options, region, &new_limit);
+		err = dmemcg_parse_limit(options, &new_limit);
 		if (err < 0)
 			goto out_put;
 

-- 
2.47.3

[PATCH v2 2/3] cgroup/dmem: accept a single region when writing to attributes

[Thadeu Lima de Souza Cascardo]

When writing to dmem.{min,low,max}, if multiple lines are given, one of
them might succeed while the next one fails, but an error is returned. That
is, there is no atomicity where either all changes succeed or all of them
fail.

Only accept a single region instead of trying to parse multiple lines and
process multiple regions at the same write.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
 kernel/cgroup/dmem.c | 69 +++++++++++++++++++++++-----------------------------
 1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
index 1ab1fb47f2711ecc60dd13e611a8a4920b48f3e9..695d2b7516081256da030c80b54ec1c5fcd6ca16 100644
--- a/kernel/cgroup/dmem.c
+++ b/kernel/cgroup/dmem.c
@@ -729,56 +729,47 @@ static ssize_t dmemcg_limit_write(struct kernfs_open_file *of,
 {
 	struct dmemcg_state *dmemcs = css_to_dmemcs(of_css(of));
 	int err = 0;
+	struct dmem_cgroup_pool_state *pool = NULL;
+	char *region_name;
+	struct dmem_cgroup_region *region;
+	u64 new_limit;
 
-	while (buf && !err) {
-		struct dmem_cgroup_pool_state *pool = NULL;
-		char *options, *region_name;
-		struct dmem_cgroup_region *region;
-		u64 new_limit;
-
-		options = buf;
-		buf = strchr(buf, '\n');
-		if (buf)
-			*buf++ = '\0';
-
-		options = strstrip(options);
+	buf = strstrip(buf);
+	if (!buf[0])
+		return -EINVAL;
 
-		/* eat empty lines */
-		if (!options[0])
-			continue;
+	region_name = strsep(&buf, " \t");
+	if (!region_name[0])
+		return -EINVAL;
 
-		region_name = strsep(&options, " \t");
-		if (!region_name[0])
-			continue;
+	if (!buf || !*buf)
+		return -EINVAL;
 
-		if (!options || !*options)
-			return -EINVAL;
+	buf = skip_spaces(buf);
 
-		rcu_read_lock();
-		region = dmemcg_get_region_by_name(region_name);
-		rcu_read_unlock();
+	err = dmemcg_parse_limit(buf, &new_limit);
+	if (err < 0)
+		return -EINVAL;
 
-		if (!region)
-			return -EINVAL;
+	rcu_read_lock();
+	region = dmemcg_get_region_by_name(region_name);
+	rcu_read_unlock();
 
-		err = dmemcg_parse_limit(options, &new_limit);
-		if (err < 0)
-			goto out_put;
+	if (!region)
+		return -EINVAL;
 
-		pool = get_cg_pool_unlocked(dmemcs, region);
-		if (IS_ERR(pool)) {
-			err = PTR_ERR(pool);
-			goto out_put;
-		}
+	pool = get_cg_pool_unlocked(dmemcs, region);
+	if (IS_ERR(pool)) {
+		err = PTR_ERR(pool);
+		goto out_put;
+	}
 
-		/* And commit */
-		apply(pool, new_limit);
-		dmemcg_pool_put(pool);
+	/* And commit */
+	apply(pool, new_limit);
+	dmemcg_pool_put(pool);
 
 out_put:
-		kref_put(&region->ref, dmemcg_free_region);
-	}
-
+	kref_put(&region->ref, dmemcg_free_region);
 
 	return err ?: nbytes;
 }

-- 
2.47.3

[PATCH v2 3/3] cgroup/dmem: allow max to be set below current usage

[Thadeu Lima de Souza Cascardo]

page_counter_set_max may return -EBUSY in case the current usage is above
the new max. When writing to dmem.max, this error is ignored and the new
max is not set.

Instead of using page_counter_set_max when writing to dmem.max, atomically
update its value irrespective of the current usage.

Since there is no current mechanism to evict a given dmemcg pool, this will
at least prevent the current usage from growing any further.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
 kernel/cgroup/dmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
index 695d2b7516081256da030c80b54ec1c5fcd6ca16..bf9e0d11e46156a437196c77fdfde84250e65420 100644
--- a/kernel/cgroup/dmem.c
+++ b/kernel/cgroup/dmem.c
@@ -159,7 +159,7 @@ set_resource_low(struct dmem_cgroup_pool_state *pool, u64 val)
 static void
 set_resource_max(struct dmem_cgroup_pool_state *pool, u64 val)
 {
-	page_counter_set_max(&pool->cnt, val);
+	xchg(&pool->cnt.max, val);
 }
 
 static u64 get_resource_low(struct dmem_cgroup_pool_state *pool)

-- 
2.47.3

Claude review: cgroup/dmem: allow atomic irrestrictive writes to dmem.max

[Claude Code Review Bot]

Overall Series Review

Subject: cgroup/dmem: allow atomic irrestrictive writes to dmem.max
Author: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Patches: 4
Reviewed: 2026-03-22T04:08:55.621521

---

This is a clean, well-motivated 3-patch series that fixes a real bug in the dmem cgroup controller: writes to `dmem.max` were silently failing when usage exceeded the new limit (because `page_counter_set_max` returns `-EBUSY` which was ignored). The series also addresses an atomicity issue with multi-region writes.

The approach is sound: simplify the write path to single-region-per-write, then replace `page_counter_set_max()` with a direct `xchg()` to unconditionally set the limit. The patches are well-ordered and each does one logical thing. The code after the series is simpler and more correct.

No significant issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: cgroup/dmem: remove region parameter from dmemcg_parse_limit

[Claude Code Review Bot]

Patch Review

**Verdict: Good**

Straightforward dead parameter removal. The `region` parameter was indeed unused in `dmemcg_parse_limit` — the function only parses `"max"` or calls `memparse()`. This is a correct preparatory cleanup.

No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: cgroup/dmem: accept a single region when writing to attributes

[Claude Code Review Bot]

Patch Review

**Verdict: Good, minor observations**

This replaces the `while` loop that iterated over newline-separated regions with a flat single-region parse. The motivation (lack of atomicity when multiple regions could partially succeed) is valid.

The control flow is clean. One observation:

- After `strstrip(buf)` at line 737, the `strsep(&buf, " \t")` call at line 741 will always return a non-empty string if `buf[0]` was non-zero (checked at line 738), so the `!region_name[0]` check at line 742 is technically redundant but harmless as defensive coding.

- The `out_put` label correctly drops the region ref via `kref_put`. The early returns before the region lookup don't need ref cleanup, and the error path after `get_cg_pool_unlocked` correctly jumps to `out_put`. The refcounting is correct.

- **Minor nit**: The patch doesn't strip a trailing newline before parsing. If userspace writes `"region_name 1024\n"`, `strstrip()` at the top handles that. But if input is `"region_name 1024\n\n"` or `"region_name 1024\nother_region 2048\n"`, the `memparse` in `dmemcg_parse_limit` would hit the `\n` and return `-EINVAL` via `*end != '\0'`. This is the desired behavior (rejecting multi-region input), but it would be slightly more user-friendly to strip trailing whitespace from `buf` after `skip_spaces`. In practice `strstrip` already strips trailing whitespace from the entire buffer, so this is fine — `buf` after `strsep` points past the separator, and `skip_spaces` handles leading whitespace, while `strstrip` already trimmed trailing whitespace from the whole original buffer. No issue here.

---
Generated by Claude Code Patch Reviewer

Claude review: cgroup/dmem: allow max to be set below current usage

[Claude Code Review Bot]

Patch Review

**Verdict: Good**

The core fix. Replaces:
```c
page_counter_set_max(&pool->cnt, val);
```
with:
```c
xchg(&pool->cnt.max, val);
```

This is correct. Looking at `page_counter_set_max()` in `mm/page_counter.c:197`, it uses an `xchg` internally but wraps it in a loop that returns `-EBUSY` when usage exceeds the new max. The return value was being silently discarded by `set_resource_max()` (which returns `void`), causing the write to appear successful while the limit remained unchanged.

The direct `xchg` unconditionally sets the max, which:
1. Has the correct memory ordering semantics (full barrier, matching what `page_counter_try_charge` expects)
2. Prevents new allocations from exceeding the new limit
3. Matches the intended memcg-like behavior where setting max below usage is allowed

One consideration: `page_counter_set_max` was designed with the read-swap-read pattern to ensure coherency with `page_counter_try_charge`. By doing a bare `xchg` without the subsequent usage check, there's a theoretical window where a concurrent charge could slip through if it read the old (higher) max just before the `xchg`. However, this is the same race that exists in memcg's `memory.max` write path (which also just does `WRITE_ONCE` on the max), and is acceptable — the charge will be caught on the next attempt. This is fine.

---
Generated by Claude Code Patch Reviewer