Re: [PATCH 4/4] cgroup: Do not report unavailable v1 controllers in /proc/cgroups

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 2398 bytes --]

On Mon, 2024-09-09 at 18:32 +0200, Michal Koutný wrote:
> This is a followup to CONFIG-urability of cpuset and memory controllers
> for v1 hierarchies. Make the output in /proc/cgroups reflect that
> !CONFIG_CPUSETS_V1 is like !CONFIG_CPUSETS and
> !CONFIG_MEMCG_V1 is like !CONFIG_MEMCG.
> 
> The intended effect is that hiding the unavailable controllers will hint
> users not to try mounting them on v1.

This change can cause problems for the OpenJDK JVM, as reported in
<https://bugs.debian.org/1108294>.

Since OpenJDK version 11, the JVM can detect and adapt to cpuset and
memory limits.  It supports both the cgroups v1 and v2 API, but before
version 25 it always relied on /proc/cgroups to detect whether those
controllers were enabled.

The result of this patch is that if CONFIG_MEMCG_V1 is disabled the JVM
can easily trigger OOM when otherwise it would trim its memory usage
through garbage collection.  (For cpusets, I'm not sure of the impact
but I think it might make bad decisions about the size of thread pools.)

Although the fix in OpenJDK 25 can probably be backported to older
versions, this issue primarily affects container workloads so fixing
this in distribution packages would not be sufficient.

The obvious compatibility fix for this at the kernel level is to enable
CONFIG_{CPUSETS,MEMCG}_V1.  But since the v1 API has long been
deprecated and is not actually needed by OpenJDK, I would prefer not to
do that.

Would you consider reverting this change for the sake of compatibility?

Ben.

> Signed-off-by: Michal Koutný <mkoutny@suse.com>
> ---
>  kernel/cgroup/cgroup-v1.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
> index 784337694a4be..e28d5f0d20ed0 100644
> --- a/kernel/cgroup/cgroup-v1.c
> +++ b/kernel/cgroup/cgroup-v1.c
> @@ -681,11 +681,14 @@ int proc_cgroupstats_show(struct seq_file *m, void *v)
>  	 * cgroup_mutex contention.
>  	 */
>  
> -	for_each_subsys(ss, i)
> +	for_each_subsys(ss, i) {
> +		if (cgroup1_subsys_absent(ss))
> +			continue;
>  		seq_printf(m, "%s\t%d\t%d\t%d\n",
>  			   ss->legacy_name, ss->root->hierarchy_id,
>  			   atomic_read(&ss->root->nr_cgrps),
>  			   cgroup_ssid_enabled(i));
> +	}
>  
>  	return 0;
>  }

-- 
Ben Hutchings
73.46% of all statistics are made up.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]