2021-10-25 06:43:58

by Shakeel Butt

Subject: [PATCH 3/3] cgroup: no need for cgroup_mutex for /proc/cgroups

On the real systems, the cgroups hierarchies are setup early and just
once by the node controller, so, other than number of cgroups, all
information in /proc/cgroups remain same for the system uptime. Let's
remove the cgroup_mutex usage on reading /proc/cgroups. There is a
chance of inconsistent number of cgroups for co-mounted cgroups while
printing the information from /proc/cgroups but that is not a big
issue. In addition /proc/cgroups is a v1 specific interface, so the
dependency on it should reduce over time.

The main motivation for removing the cgroup_mutex from /proc/cgroups is
to reduce the avenues of its contention. On our fleet, we have observed
buggy application hammering on /proc/cgroups and drastically slowing
down the node controller on the system which have many negative
consequences on other workloads running on the system.

Signed-off-by: Shakeel Butt <[email protected]>
---
kernel/cgroup/cgroup-v1.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index fd14a60379c1..81c9e0685948 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -659,11 +659,9 @@ int proc_cgroupstats_show(struct seq_file *m, void *v)

seq_puts(m, "#subsys_name\thierarchy\tnum_cgroups\tenabled\n");
/*
- * ideally we don't want subsystems moving around while we do this.
- * cgroup_mutex is also necessary to guarantee an atomic snapshot of
- * subsys/hierarchy state.
+ * Grab the subsystems state racily. No need to add avenue to
+ * cgroup_mutex contention.
*/
- mutex_lock(&cgroup_mutex);

for_each_subsys(ss, i)
seq_printf(m, "%s\t%d\t%d\t%d\n",
@@ -671,7 +669,6 @@ int proc_cgroupstats_show(struct seq_file *m, void *v)
atomic_read(&ss->root->nr_cgrps),
cgroup_ssid_enabled(i));

- mutex_unlock(&cgroup_mutex);
return 0;
}

--
2.33.0.1079.g6e70778dc9-goog

2021-10-25 17:30:30

by Tejun Heo

Subject: Re: [PATCH 3/3] cgroup: no need for cgroup_mutex for /proc/cgroups

On Sun, Oct 24, 2021 at 11:19:16PM -0700, Shakeel Butt wrote:
> On the real systems, the cgroups hierarchies are setup early and just
> once by the node controller, so, other than number of cgroups, all
> information in /proc/cgroups remain same for the system uptime. Let's
> remove the cgroup_mutex usage on reading /proc/cgroups. There is a
> chance of inconsistent number of cgroups for co-mounted cgroups while
> printing the information from /proc/cgroups but that is not a big
> issue. In addition /proc/cgroups is a v1 specific interface, so the
> dependency on it should reduce over time.
>
> The main motivation for removing the cgroup_mutex from /proc/cgroups is
> to reduce the avenues of its contention. On our fleet, we have observed
> buggy application hammering on /proc/cgroups and drastically slowing
> down the node controller on the system which have many negative
> consequences on other workloads running on the system.
>
> Signed-off-by: Shakeel Butt <[email protected]>

Applied 1-3 to cgroup/for-5.16.

Thanks.

--
tejun