On Mon, Jun 08, 2026 at 11:53:51AM -0400, Eric Chanudet wrote: > Accept only one "region value" pair entry for the dmem.max, dmem.min, > dmem.low files. > > This changes the UAPI that otherwise accepted multiple lines for setting > multiple entries in one write. No existing user is known to rely on > writing multiple regions in a single write. > > Processing multiple regions in dmemcg_limit_write() could quietly change > first limits before failing on a later one and returning an error to the > writer, with no indication some changes occurred. > > Acked-by: Tejun Heo <[email protected]> > Signed-off-by: Eric Chanudet <[email protected]>
I did some review over any potential NULL derefs and tested with different corner cases. I could not find any issues. Thanks. Cascardo. Reviewed-by: Thadeu Lima de Souza Cascardo <[email protected]> Tested-by: Thadeu Lima de Souza Cascardo <[email protected]> > --- > Follow up from discussions on a previous thread[1]. > If Albert's series[2] lands, I can cleanup and prepare some kunits for > these as well. > [1] > https://lore.kernel.org/all/[email protected]/ > [2] > https://lore.kernel.org/all/[email protected]/ > --- > Changes in v2: > - Handle buf == NULL by testing !buf first after strsep (Natalie) > - Don't allow extra spaces to separate key and value (Natalie) > Other cgroup files don't (rdma, misc), so stay consistent. > - Link to v1: > https://lore.kernel.org/r/20260605-cgroup-dmem-write-single-region-v1-1-9137f2965...@redhat.com > --- > kernel/cgroup/dmem.c | 69 > +++++++++++++++++++--------------------------------- > 1 file changed, 25 insertions(+), 44 deletions(-) > > diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c > index > 6430c7ce1e0372f59f1313163fb7630ce49ac1ef..39930c59cb769a505a5852a5644a371fd5596f59 > 100644 > --- a/kernel/cgroup/dmem.c > +++ b/kernel/cgroup/dmem.c > @@ -734,57 +734,38 @@ static ssize_t dmemcg_limit_write(struct > kernfs_open_file *of, > void (*apply)(struct dmem_cgroup_pool_state *, > u64)) > { > struct dmemcg_state *dmemcs = css_to_dmemcs(of_css(of)); > - int err = 0; > - > - 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); > - > - /* eat empty lines */ > - if (!options[0]) > - continue; > - > - region_name = strsep(&options, " \t"); > - if (!region_name[0]) > - continue; > - > - if (!options || !*options) > - return -EINVAL; > + struct dmem_cgroup_pool_state *pool; > + struct dmem_cgroup_region *region; > + char *region_name; > + u64 new_limit; > + int err; > > - rcu_read_lock(); > - region = dmemcg_get_region_by_name(region_name); > - rcu_read_unlock(); > + buf = strstrip(buf); > + region_name = strsep(&buf, " \t"); > + if (!buf || !region_name[0]) > + return -EINVAL; > > - if (!region) > - return -EINVAL; > + rcu_read_lock(); > + region = dmemcg_get_region_by_name(region_name); > + rcu_read_unlock(); > + if (!region) > + return -EINVAL; > > - 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); > + 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); > > out_put: > - kref_put(®ion->ref, dmemcg_free_region); > - } > - > + kref_put(®ion->ref, dmemcg_free_region); > > return err ?: nbytes; > } > > --- > base-commit: 640c57d6ca1346a1c2363a3f473b405af979e046 > change-id: 20260605-cgroup-dmem-write-single-region-9bf05b6d995d > > Best regards, > -- > Eric Chanudet <[email protected]> >
