| Message ID | 20250408073243.488-2-rakie.kim@sk.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enhance sysfs handling for memory hotplug in weighted interleave | expand |
On Tue, 8 Apr 2025 16:32:40 +0900 Rakie Kim <rakie.kim@sk.com> wrote: Hi Rakie, Thank you for your work on this fix! Everything looks good to me : -) Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com> > Memory leaks occurred when removing sysfs attributes for weighted > interleave. Improper kobject deallocation led to unreleased memory > when initialization failed or when nodes were removed. > > This patch resolves the issue by replacing unnecessary `kfree()` > calls with proper `kobject_del()` and `kobject_put()` sequences, > ensuring correct teardown and preventing memory leaks. > > By explicitly calling `kobject_del()` before `kobject_put()`, > the release function is now invoked safely, and internal sysfs > state is correctly cleaned up. This guarantees that the memory > associated with the kobject is fully released and avoids > resource leaks, thereby improving system stability. > > Fixes: dce41f5ae253 ("mm/mempolicy: implement the sysfs-based weighted_interleave interface") > Signed-off-by: Rakie Kim <rakie.kim@sk.com> > Signed-off-by: Honggyu Kim <honggyu.kim@sk.com> > Signed-off-by: Yunjeong Mun <yunjeong.mun@sk.com> > Reviewed-by: Gregory Price <gourry@gourry.net> > --- > mm/mempolicy.c | 66 ++++++++++++++++++++++++-------------------------- > 1 file changed, 32 insertions(+), 34 deletions(-) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index b28a1e6ae096..0da102aa1cfc 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -3479,7 +3479,9 @@ static void sysfs_wi_release(struct kobject *wi_kobj) > > for (i = 0; i < nr_node_ids; i++) > sysfs_wi_node_release(node_attrs[i], wi_kobj); > - kobject_put(wi_kobj); > + > + kfree(node_attrs); > + kfree(wi_kobj); > } > > static const struct kobj_type wi_ktype = { > @@ -3525,27 +3527,37 @@ static int add_weighted_interleave_group(struct kobject *root_kobj) > struct kobject *wi_kobj; > int nid, err; > > + node_attrs = kcalloc(nr_node_ids, sizeof(struct iw_node_attr *), > + GFP_KERNEL); > + if (!node_attrs) > + return -ENOMEM; > + > wi_kobj = kzalloc(sizeof(struct kobject), GFP_KERNEL); > - if (!wi_kobj) > + if (!wi_kobj) { > + kfree(node_attrs); > return -ENOMEM; > + } > > err = kobject_init_and_add(wi_kobj, &wi_ktype, root_kobj, > "weighted_interleave"); > - if (err) { > - kfree(wi_kobj); > - return err; > - } > + if (err) > + goto err_put_kobj; > > for_each_node_state(nid, N_POSSIBLE) { > err = add_weight_node(nid, wi_kobj); > if (err) { > pr_err("failed to add sysfs [node%d]\n", nid); > - break; > + goto err_del_kobj; > } > } > - if (err) > - kobject_put(wi_kobj); > + > return 0; > + > +err_del_kobj: > + kobject_del(wi_kobj); > +err_put_kobj: > + kobject_put(wi_kobj); > + return err; > } > > static void mempolicy_kobj_release(struct kobject *kobj) > @@ -3559,7 +3571,6 @@ static void mempolicy_kobj_release(struct kobject *kobj) > mutex_unlock(&iw_table_lock); > synchronize_rcu(); > kfree(old); > - kfree(node_attrs); > kfree(kobj); > } > > @@ -3573,37 +3584,24 @@ static int __init mempolicy_sysfs_init(void) > static struct kobject *mempolicy_kobj; > > mempolicy_kobj = kzalloc(sizeof(*mempolicy_kobj), GFP_KERNEL); > - if (!mempolicy_kobj) { > - err = -ENOMEM; > - goto err_out; > - } > - > - node_attrs = kcalloc(nr_node_ids, sizeof(struct iw_node_attr *), > - GFP_KERNEL); > - if (!node_attrs) { > - err = -ENOMEM; > - goto mempol_out; > - } > + if (!mempolicy_kobj) > + return -ENOMEM; > > err = kobject_init_and_add(mempolicy_kobj, &mempolicy_ktype, mm_kobj, > "mempolicy"); > if (err) > - goto node_out; > + goto err_put_kobj; > > err = add_weighted_interleave_group(mempolicy_kobj); > - if (err) { > - pr_err("mempolicy sysfs structure failed to initialize\n"); > - kobject_put(mempolicy_kobj); > - return err; > - } > + if (err) > + goto err_del_kobj; > > - return err; > -node_out: > - kfree(node_attrs); > -mempol_out: > - kfree(mempolicy_kobj); > -err_out: > - pr_err("failed to add mempolicy kobject to the system\n"); > + return 0; > + > +err_del_kobj: > + kobject_del(mempolicy_kobj); > +err_put_kobj: > + kobject_put(mempolicy_kobj); > return err; > } > > -- > 2.34.1 Sent using hkml (https://github.com/sjp38/hackermail)
On Tue, 8 Apr 2025 16:32:40 +0900 Rakie Kim <rakie.kim@sk.com> wrote: > Memory leaks occurred when removing sysfs attributes for weighted > interleave. Improper kobject deallocation led to unreleased memory > when initialization failed or when nodes were removed. > > This patch resolves the issue by replacing unnecessary `kfree()` > calls with proper `kobject_del()` and `kobject_put()` sequences, > ensuring correct teardown and preventing memory leaks. > > By explicitly calling `kobject_del()` before `kobject_put()`, > the release function is now invoked safely, and internal sysfs > state is correctly cleaned up. This guarantees that the memory > associated with the kobject is fully released and avoids > resource leaks, thereby improving system stability. > > Fixes: dce41f5ae253 ("mm/mempolicy: implement the sysfs-based weighted_interleave interface") > Signed-off-by: Rakie Kim <rakie.kim@sk.com> > Signed-off-by: Honggyu Kim <honggyu.kim@sk.com> > Signed-off-by: Yunjeong Mun <yunjeong.mun@sk.com> > Reviewed-by: Gregory Price <gourry@gourry.net> LGTM Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
diff --git a/mm/mempolicy.c b/mm/mempolicy.c index b28a1e6ae096..0da102aa1cfc 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -3479,7 +3479,9 @@ static void sysfs_wi_release(struct kobject *wi_kobj) for (i = 0; i < nr_node_ids; i++) sysfs_wi_node_release(node_attrs[i], wi_kobj); - kobject_put(wi_kobj); + + kfree(node_attrs); + kfree(wi_kobj); } static const struct kobj_type wi_ktype = { @@ -3525,27 +3527,37 @@ static int add_weighted_interleave_group(struct kobject *root_kobj) struct kobject *wi_kobj; int nid, err; + node_attrs = kcalloc(nr_node_ids, sizeof(struct iw_node_attr *), + GFP_KERNEL); + if (!node_attrs) + return -ENOMEM; + wi_kobj = kzalloc(sizeof(struct kobject), GFP_KERNEL); - if (!wi_kobj) + if (!wi_kobj) { + kfree(node_attrs); return -ENOMEM; + } err = kobject_init_and_add(wi_kobj, &wi_ktype, root_kobj, "weighted_interleave"); - if (err) { - kfree(wi_kobj); - return err; - } + if (err) + goto err_put_kobj; for_each_node_state(nid, N_POSSIBLE) { err = add_weight_node(nid, wi_kobj); if (err) { pr_err("failed to add sysfs [node%d]\n", nid); - break; + goto err_del_kobj; } } - if (err) - kobject_put(wi_kobj); + return 0; + +err_del_kobj: + kobject_del(wi_kobj); +err_put_kobj: + kobject_put(wi_kobj); + return err; } static void mempolicy_kobj_release(struct kobject *kobj) @@ -3559,7 +3571,6 @@ static void mempolicy_kobj_release(struct kobject *kobj) mutex_unlock(&iw_table_lock); synchronize_rcu(); kfree(old); - kfree(node_attrs); kfree(kobj); } @@ -3573,37 +3584,24 @@ static int __init mempolicy_sysfs_init(void) static struct kobject *mempolicy_kobj; mempolicy_kobj = kzalloc(sizeof(*mempolicy_kobj), GFP_KERNEL); - if (!mempolicy_kobj) { - err = -ENOMEM; - goto err_out; - } - - node_attrs = kcalloc(nr_node_ids, sizeof(struct iw_node_attr *), - GFP_KERNEL); - if (!node_attrs) { - err = -ENOMEM; - goto mempol_out; - } + if (!mempolicy_kobj) + return -ENOMEM; err = kobject_init_and_add(mempolicy_kobj, &mempolicy_ktype, mm_kobj, "mempolicy"); if (err) - goto node_out; + goto err_put_kobj; err = add_weighted_interleave_group(mempolicy_kobj); - if (err) { - pr_err("mempolicy sysfs structure failed to initialize\n"); - kobject_put(mempolicy_kobj); - return err; - } + if (err) + goto err_del_kobj; - return err; -node_out: - kfree(node_attrs); -mempol_out: - kfree(mempolicy_kobj); -err_out: - pr_err("failed to add mempolicy kobject to the system\n"); + return 0; + +err_del_kobj: + kobject_del(mempolicy_kobj); +err_put_kobj: + kobject_put(mempolicy_kobj); return err; }
Memory leaks occurred when removing sysfs attributes for weighted interleave. Improper kobject deallocation led to unreleased memory when initialization failed or when nodes were removed. This patch resolves the issue by replacing unnecessary `kfree()` calls with proper `kobject_del()` and `kobject_put()` sequences, ensuring correct teardown and preventing memory leaks. By explicitly calling `kobject_del()` before `kobject_put()`, the release function is now invoked safely, and internal sysfs state is correctly cleaned up. This guarantees that the memory associated with the kobject is fully released and avoids resource leaks, thereby improving system stability. Fixes: dce41f5ae253 ("mm/mempolicy: implement the sysfs-based weighted_interleave interface") Signed-off-by: Rakie Kim <rakie.kim@sk.com> Signed-off-by: Honggyu Kim <honggyu.kim@sk.com> Signed-off-by: Yunjeong Mun <yunjeong.mun@sk.com> Reviewed-by: Gregory Price <gourry@gourry.net> --- mm/mempolicy.c | 66 ++++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 34 deletions(-)