Skip to content

Commit fd8d9db

Browse files
xiaochenshensuryasaimadhu
authored andcommitted
x86/resctrl: Remove superfluous kernfs_get() calls to prevent refcount leak
Willem reported growing of kernfs_node_cache entries in slabtop when repeatedly creating and removing resctrl subdirectories as well as when repeatedly mounting and unmounting the resctrl filesystem. On resource group (control as well as monitoring) creation via a mkdir an extra kernfs_node reference is obtained to ensure that the rdtgroup structure remains accessible for the rdtgroup_kn_unlock() calls where it is removed on deletion. The kernfs_node reference count is dropped by kernfs_put() in rdtgroup_kn_unlock(). With the above explaining the need for one kernfs_get()/kernfs_put() pair in resctrl there are more places where a kernfs_node reference is obtained without a corresponding release. The excessive amount of reference count on kernfs nodes will never be dropped to 0 and the kernfs nodes will never be freed in the call paths of rmdir and umount. It leads to reference count leak and kernfs_node_cache memory leak. Remove the superfluous kernfs_get() calls and expand the existing comments surrounding the remaining kernfs_get()/kernfs_put() pair that remains in use. Superfluous kernfs_get() calls are removed from two areas: (1) In call paths of mount and mkdir, when kernfs nodes for "info", "mon_groups" and "mon_data" directories and sub-directories are created, the reference count of newly created kernfs node is set to 1. But after kernfs_create_dir() returns, superfluous kernfs_get() are called to take an additional reference. (2) kernfs_get() calls in rmdir call paths. Fixes: 17eafd0 ("x86/intel_rdt: Split resource group removal in two") Fixes: 4af4a88 ("x86/intel_rdt/cqm: Add mount,umount support") Fixes: f3cbeac ("x86/intel_rdt/cqm: Add rmdir support") Fixes: d89b737 ("x86/intel_rdt/cqm: Add mon_data") Fixes: c7d9aac ("x86/intel_rdt/cqm: Add mkdir support for RDT monitoring") Fixes: 5dc1d5c ("x86/intel_rdt: Simplify info and base file lists") Fixes: 60cf5e1 ("x86/intel_rdt: Add mkdir to resctrl file system") Fixes: 4e978d0 ("x86/intel_rdt: Add "info" files to resctrl file system") Reported-by: Willem de Bruijn <willemb@google.com> Signed-off-by: Xiaochen Shen <xiaochen.shen@intel.com> Signed-off-by: Borislav Petkov <bp@suse.de> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Tested-by: Willem de Bruijn <willemb@google.com> Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/1604085053-31639-1-git-send-email-xiaochen.shen@intel.com
1 parent 418baf2 commit fd8d9db

1 file changed

Lines changed: 2 additions & 33 deletions

File tree

arch/x86/kernel/cpu/resctrl/rdtgroup.c

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,7 +1769,6 @@ static int rdtgroup_mkdir_info_resdir(struct rdt_resource *r, char *name,
17691769
if (IS_ERR(kn_subdir))
17701770
return PTR_ERR(kn_subdir);
17711771

1772-
kernfs_get(kn_subdir);
17731772
ret = rdtgroup_kn_set_ugid(kn_subdir);
17741773
if (ret)
17751774
return ret;
@@ -1792,7 +1791,6 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
17921791
kn_info = kernfs_create_dir(parent_kn, "info", parent_kn->mode, NULL);
17931792
if (IS_ERR(kn_info))
17941793
return PTR_ERR(kn_info);
1795-
kernfs_get(kn_info);
17961794

17971795
ret = rdtgroup_add_files(kn_info, RF_TOP_INFO);
17981796
if (ret)
@@ -1813,12 +1811,6 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
18131811
goto out_destroy;
18141812
}
18151813

1816-
/*
1817-
* This extra ref will be put in kernfs_remove() and guarantees
1818-
* that @rdtgrp->kn is always accessible.
1819-
*/
1820-
kernfs_get(kn_info);
1821-
18221814
ret = rdtgroup_kn_set_ugid(kn_info);
18231815
if (ret)
18241816
goto out_destroy;
@@ -1847,12 +1839,6 @@ mongroup_create_dir(struct kernfs_node *parent_kn, struct rdtgroup *prgrp,
18471839
if (dest_kn)
18481840
*dest_kn = kn;
18491841

1850-
/*
1851-
* This extra ref will be put in kernfs_remove() and guarantees
1852-
* that @rdtgrp->kn is always accessible.
1853-
*/
1854-
kernfs_get(kn);
1855-
18561842
ret = rdtgroup_kn_set_ugid(kn);
18571843
if (ret)
18581844
goto out_destroy;
@@ -2139,13 +2125,11 @@ static int rdt_get_tree(struct fs_context *fc)
21392125
&kn_mongrp);
21402126
if (ret < 0)
21412127
goto out_info;
2142-
kernfs_get(kn_mongrp);
21432128

21442129
ret = mkdir_mondata_all(rdtgroup_default.kn,
21452130
&rdtgroup_default, &kn_mondata);
21462131
if (ret < 0)
21472132
goto out_mongrp;
2148-
kernfs_get(kn_mondata);
21492133
rdtgroup_default.mon.mon_data_kn = kn_mondata;
21502134
}
21512135

@@ -2499,11 +2483,6 @@ static int mkdir_mondata_subdir(struct kernfs_node *parent_kn,
24992483
if (IS_ERR(kn))
25002484
return PTR_ERR(kn);
25012485

2502-
/*
2503-
* This extra ref will be put in kernfs_remove() and guarantees
2504-
* that kn is always accessible.
2505-
*/
2506-
kernfs_get(kn);
25072486
ret = rdtgroup_kn_set_ugid(kn);
25082487
if (ret)
25092488
goto out_destroy;
@@ -2838,8 +2817,8 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
28382817
/*
28392818
* kernfs_remove() will drop the reference count on "kn" which
28402819
* will free it. But we still need it to stick around for the
2841-
* rdtgroup_kn_unlock(kn} call below. Take one extra reference
2842-
* here, which will be dropped inside rdtgroup_kn_unlock().
2820+
* rdtgroup_kn_unlock(kn) call. Take one extra reference here,
2821+
* which will be dropped inside rdtgroup_kn_unlock().
28432822
*/
28442823
kernfs_get(kn);
28452824

@@ -3049,11 +3028,6 @@ static int rdtgroup_rmdir_mon(struct kernfs_node *kn, struct rdtgroup *rdtgrp,
30493028
WARN_ON(list_empty(&prdtgrp->mon.crdtgrp_list));
30503029
list_del(&rdtgrp->mon.crdtgrp_list);
30513030

3052-
/*
3053-
* one extra hold on this, will drop when we kfree(rdtgrp)
3054-
* in rdtgroup_kn_unlock()
3055-
*/
3056-
kernfs_get(kn);
30573031
kernfs_remove(rdtgrp->kn);
30583032

30593033
return 0;
@@ -3065,11 +3039,6 @@ static int rdtgroup_ctrl_remove(struct kernfs_node *kn,
30653039
rdtgrp->flags = RDT_DELETED;
30663040
list_del(&rdtgrp->rdtgroup_list);
30673041

3068-
/*
3069-
* one extra hold on this, will drop when we kfree(rdtgrp)
3070-
* in rdtgroup_kn_unlock()
3071-
*/
3072-
kernfs_get(kn);
30733042
kernfs_remove(rdtgrp->kn);
30743043
return 0;
30753044
}

0 commit comments

Comments
 (0)