Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Infinite spin loop in `get_cg_pool_unlocked` if memory allocation fails under memory pressure. -- commit 04831a5f7efe525cb1eb2645936fd7233c21b4bf Author: Eric Chanudet <[email protected]> Subject: cgroup/dmem: accept only one region per limit write This patch changes the UAPI for dmem.max, dmem.min, and dmem.low to accept only one region value pair entry per write. This simplifies the logic and prevents partial limit applications that could quietly change state before failing on a later entry. > diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c > index 1ab1fb47f2711..542a5d387b551 100644 > --- a/kernel/cgroup/dmem.c > +++ b/kernel/cgroup/dmem.c [ ... ] > @@ -728,57 +728,38 @@ static ssize_t dmemcg_limit_write(struct > kernfs_open_file *of, [ ... ] > - err = dmemcg_parse_limit(options, &new_limit); > - if (err < 0) > - goto out_put; > + err = dmemcg_parse_limit(buf, &new_limit); > + if (err < 0) > + goto out_put; > > - 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); [Severity: High] This is a pre-existing issue, but while reviewing this call to get_cg_pool_unlocked(), I noticed it might spin infinitely under memory pressure instead of returning an error. Looking at the allocation fallback inside get_cg_pool_unlocked(): kernel/cgroup/dmem.c:get_cg_pool_unlocked() { ... while (!pool) { ... if (pool == ERR_PTR(-ENOMEM)) { pool = NULL; if (WARN_ON(allocpool)) continue; allocpool = kzalloc_obj(*allocpool); if (allocpool) { pool = NULL; continue; } } } ... } If the system is out of memory and kzalloc_obj() returns NULL, the loop condition while (!pool) remains true because pool was explicitly set to NULL right before the allocation attempt. Will this continuously loop and re-acquire the global spinlock without ever propagating the -ENOMEM error back to the caller here? > + if (IS_ERR(pool)) { > + err = PTR_ERR(pool); > + goto out_put; > + } > > - /* And commit */ > - apply(pool, new_limit); > - dmemcg_pool_put(pool); > + apply(pool, new_limit); > + dmemcg_pool_put(pool); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260608-cgroup-dmem-write-single-region-v2-1-b0cd6c4cc...@redhat.com?part=1
