| Message ID | 20220816130553.31406-6-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few fixup patches for hugetlb | expand |
Hi Miaohe, On 8/16/2022 9:05 PM, Miaohe Lin wrote: > } > > if (h->demote_order) { > - if (sysfs_create_group(hstate_kobjs[hi], > - &hstate_demote_attr_group)) > + retval = sysfs_create_group(hstate_kobjs[hi], > + &hstate_demote_attr_group); What about add one more: just return if hstate_attr_group creating failed: diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 0aee2f3ae15c..a67ef4b4eb3f 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -3845,6 +3845,7 @@ static int hugetlb_sysfs_add_hstate(struct hstate *h, struct kobject *parent, if (retval) { kobject_put(hstate_kobjs[hi]); hstate_kobjs[hi] = NULL; + return retval; } Once hstate_kobjs[hi] is set to NULL, hstate_demote_attr_group creating will fail as well. Thanks. Regards Yin, Fengwei > + if (retval) { > pr_warn("HugeTLB unable to create demote interfaces for %s\n", h->name); > + sysfs_remove_group(hstate_kobjs[hi], hstate_attr_group); > + kobject_put(hstate_kobjs[hi]); > + hstate_kobjs[hi] = NULL; > + return retval; > + } > }
On 8/17/2022 5:41 PM, Yin, Fengwei wrote: > Hi Miaohe, > > On 8/16/2022 9:05 PM, Miaohe Lin wrote: >> } >> >> if (h->demote_order) { >> - if (sysfs_create_group(hstate_kobjs[hi], >> - &hstate_demote_attr_group)) >> + retval = sysfs_create_group(hstate_kobjs[hi], >> + &hstate_demote_attr_group); > What about add one more: > just return if hstate_attr_group creating failed: > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 0aee2f3ae15c..a67ef4b4eb3f 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -3845,6 +3845,7 @@ static int hugetlb_sysfs_add_hstate(struct hstate *h, struct kobject *parent, > if (retval) { > kobject_put(hstate_kobjs[hi]); > hstate_kobjs[hi] = NULL; > + return retval; > } Please ignore this. Just saw the patch 2 made this change. Regards Yin, Fengwei > > Once hstate_kobjs[hi] is set to NULL, hstate_demote_attr_group creating will > fail as well. Thanks. > > > Regards > Yin, Fengwei > >> + if (retval) { >> pr_warn("HugeTLB unable to create demote interfaces for %s\n", h->name); >> + sysfs_remove_group(hstate_kobjs[hi], hstate_attr_group); >> + kobject_put(hstate_kobjs[hi]); >> + hstate_kobjs[hi] = NULL; >> + return retval; >> + } >> } >
On 8/16/2022 9:05 PM, Miaohe Lin wrote: > The sysfs group per_node_hstate_attr_group and hstate_demote_attr_group > when h->demote_order != 0 are created in hugetlb_register_node(). But > these sysfs groups are not removed when unregister the node, thus sysfs > group is leaked. Using sysfs_remove_group() to fix this issue. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> Reviewed-by: Fengwei Yin <fengwei.yin@intel.com> Regards Yin, Fengwei > --- > mm/hugetlb.c | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index b69d7808f457..e1356ad57087 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -3850,12 +3850,18 @@ static int hugetlb_sysfs_add_hstate(struct hstate *h, struct kobject *parent, > } > > if (h->demote_order) { > - if (sysfs_create_group(hstate_kobjs[hi], > - &hstate_demote_attr_group)) > + retval = sysfs_create_group(hstate_kobjs[hi], > + &hstate_demote_attr_group); > + if (retval) { > pr_warn("HugeTLB unable to create demote interfaces for %s\n", h->name); > + sysfs_remove_group(hstate_kobjs[hi], hstate_attr_group); > + kobject_put(hstate_kobjs[hi]); > + hstate_kobjs[hi] = NULL; > + return retval; > + } > } > > - return retval; > + return 0; > } > > static void __init hugetlb_sysfs_init(void) > @@ -3941,10 +3947,15 @@ static void hugetlb_unregister_node(struct node *node) > > for_each_hstate(h) { > int idx = hstate_index(h); > - if (nhs->hstate_kobjs[idx]) { > - kobject_put(nhs->hstate_kobjs[idx]); > - nhs->hstate_kobjs[idx] = NULL; > - } > + struct kobject *hstate_kobj = nhs->hstate_kobjs[idx]; > + > + if (!hstate_kobj) > + continue; > + if (h->demote_order) > + sysfs_remove_group(hstate_kobj, &hstate_demote_attr_group); > + sysfs_remove_group(hstate_kobj, &per_node_hstate_attr_group); > + kobject_put(hstate_kobj); > + nhs->hstate_kobjs[idx] = NULL; > } > > kobject_put(nhs->hugepages_kobj);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index b69d7808f457..e1356ad57087 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -3850,12 +3850,18 @@ static int hugetlb_sysfs_add_hstate(struct hstate *h, struct kobject *parent, } if (h->demote_order) { - if (sysfs_create_group(hstate_kobjs[hi], - &hstate_demote_attr_group)) + retval = sysfs_create_group(hstate_kobjs[hi], + &hstate_demote_attr_group); + if (retval) { pr_warn("HugeTLB unable to create demote interfaces for %s\n", h->name); + sysfs_remove_group(hstate_kobjs[hi], hstate_attr_group); + kobject_put(hstate_kobjs[hi]); + hstate_kobjs[hi] = NULL; + return retval; + } } - return retval; + return 0; } static void __init hugetlb_sysfs_init(void) @@ -3941,10 +3947,15 @@ static void hugetlb_unregister_node(struct node *node) for_each_hstate(h) { int idx = hstate_index(h); - if (nhs->hstate_kobjs[idx]) { - kobject_put(nhs->hstate_kobjs[idx]); - nhs->hstate_kobjs[idx] = NULL; - } + struct kobject *hstate_kobj = nhs->hstate_kobjs[idx]; + + if (!hstate_kobj) + continue; + if (h->demote_order) + sysfs_remove_group(hstate_kobj, &hstate_demote_attr_group); + sysfs_remove_group(hstate_kobj, &per_node_hstate_attr_group); + kobject_put(hstate_kobj); + nhs->hstate_kobjs[idx] = NULL; } kobject_put(nhs->hugepages_kobj);
The sysfs group per_node_hstate_attr_group and hstate_demote_attr_group when h->demote_order != 0 are created in hugetlb_register_node(). But these sysfs groups are not removed when unregister the node, thus sysfs group is leaked. Using sysfs_remove_group() to fix this issue. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/hugetlb.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)