[PATCH -next] cgroup: Use descriptor table to unify mount flag management

[PATCH -next] cgroup: Use descriptor table to unify mount flag management

[Chen Ridong]

From: Chen Ridong <chenridong@huawei.com>

The cgroup2 mount flags (e.g. nsdelegate, favordynmods) were previously
handled via scattered switch-case and conditional checks across
parameter parsing, flag application, and option display paths. This
leads to redundant code and increased maintenance cost when adding/removing
flags.

Introduce a `cgroup_mount_flag_desc` descriptor table to centralize the
mapping between flag bits, names, and apply functions. Refactor the
relevant paths to use this table for unified management:
1. cgroup2_parse_param: Replace switch-case with table lookup
2. apply_cgroup_root_flags: Replace multiple conditionals with table
   iteration
3. cgroup_show_options: Replace hardcoded seq_puts with table-driven output

No functional change intended, and the mount option output format remains
compatible with the original implementation.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cgroup.c | 107 +++++++++++++++++++----------------------
 1 file changed, 49 insertions(+), 58 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index e717208cfb18..1e4033d05c29 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2005,6 +2005,36 @@ static const struct fs_parameter_spec cgroup2_fs_parameters[] = {
 	{}
 };
 
+struct cgroup_mount_flag_desc {
+	u64 flag;
+	const char *name;
+	void (*apply)(struct cgroup_root *root, u64 bit, bool enable);
+};
+
+static void apply_cgroup_favor_flags(struct cgroup_root *root,
+					     u64 bit, bool enable)
+{
+	return cgroup_favor_dynmods(root, enable);
+}
+
+static void __apply_cgroup_root_flags(struct cgroup_root *root,
+					      u64 bit, bool enable)
+{
+	if (enable)
+		root->flags |= bit;
+	else
+		root->flags &= ~bit;
+}
+
+static const struct cgroup_mount_flag_desc mount_flags_desc[nr__cgroup2_params] = {
+{CGRP_ROOT_NS_DELEGATE, "nsdelegate", __apply_cgroup_root_flags},
+{CGRP_ROOT_FAVOR_DYNMODS, "favordynmods", apply_cgroup_favor_flags},
+{CGRP_ROOT_MEMORY_LOCAL_EVENTS, "memory_localevents", __apply_cgroup_root_flags},
+{CGRP_ROOT_MEMORY_RECURSIVE_PROT, "memory_recursiveprot", __apply_cgroup_root_flags},
+{CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING, "memory_hugetlb_accounting", __apply_cgroup_root_flags},
+{CGRP_ROOT_PIDS_LOCAL_EVENTS, "pids_localevents", __apply_cgroup_root_flags}
+};
+
 static int cgroup2_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
 	struct cgroup_fs_context *ctx = cgroup_fc2context(fc);
@@ -2014,28 +2044,11 @@ static int cgroup2_parse_param(struct fs_context *fc, struct fs_parameter *param
 	opt = fs_parse(fc, cgroup2_fs_parameters, param, &result);
 	if (opt < 0)
 		return opt;
+	if (opt >= nr__cgroup2_params)
+		return -EINVAL;
 
-	switch (opt) {
-	case Opt_nsdelegate:
-		ctx->flags |= CGRP_ROOT_NS_DELEGATE;
-		return 0;
-	case Opt_favordynmods:
-		ctx->flags |= CGRP_ROOT_FAVOR_DYNMODS;
-		return 0;
-	case Opt_memory_localevents:
-		ctx->flags |= CGRP_ROOT_MEMORY_LOCAL_EVENTS;
-		return 0;
-	case Opt_memory_recursiveprot:
-		ctx->flags |= CGRP_ROOT_MEMORY_RECURSIVE_PROT;
-		return 0;
-	case Opt_memory_hugetlb_accounting:
-		ctx->flags |= CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING;
-		return 0;
-	case Opt_pids_localevents:
-		ctx->flags |= CGRP_ROOT_PIDS_LOCAL_EVENTS;
-		return 0;
-	}
-	return -EINVAL;
+	ctx->flags |= mount_flags_desc[opt].flag;
+	return 0;
 }
 
 struct cgroup_of_peak *of_peak(struct kernfs_open_file *of)
@@ -2047,51 +2060,29 @@ struct cgroup_of_peak *of_peak(struct kernfs_open_file *of)
 
 static void apply_cgroup_root_flags(unsigned int root_flags)
 {
-	if (current->nsproxy->cgroup_ns == &init_cgroup_ns) {
-		if (root_flags & CGRP_ROOT_NS_DELEGATE)
-			cgrp_dfl_root.flags |= CGRP_ROOT_NS_DELEGATE;
-		else
-			cgrp_dfl_root.flags &= ~CGRP_ROOT_NS_DELEGATE;
-
-		cgroup_favor_dynmods(&cgrp_dfl_root,
-				     root_flags & CGRP_ROOT_FAVOR_DYNMODS);
-
-		if (root_flags & CGRP_ROOT_MEMORY_LOCAL_EVENTS)
-			cgrp_dfl_root.flags |= CGRP_ROOT_MEMORY_LOCAL_EVENTS;
-		else
-			cgrp_dfl_root.flags &= ~CGRP_ROOT_MEMORY_LOCAL_EVENTS;
+	int i;
 
-		if (root_flags & CGRP_ROOT_MEMORY_RECURSIVE_PROT)
-			cgrp_dfl_root.flags |= CGRP_ROOT_MEMORY_RECURSIVE_PROT;
-		else
-			cgrp_dfl_root.flags &= ~CGRP_ROOT_MEMORY_RECURSIVE_PROT;
+	if (current->nsproxy->cgroup_ns != &init_cgroup_ns)
+		return;
 
-		if (root_flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING)
-			cgrp_dfl_root.flags |= CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING;
-		else
-			cgrp_dfl_root.flags &= ~CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING;
+	for (i = 0; i < nr__cgroup2_params; ++i) {
+		bool enable;
 
-		if (root_flags & CGRP_ROOT_PIDS_LOCAL_EVENTS)
-			cgrp_dfl_root.flags |= CGRP_ROOT_PIDS_LOCAL_EVENTS;
-		else
-			cgrp_dfl_root.flags &= ~CGRP_ROOT_PIDS_LOCAL_EVENTS;
+		enable = root_flags & mount_flags_desc[i].flag;
+		mount_flags_desc[i].apply(&cgrp_dfl_root, mount_flags_desc[i].flag, enable);
 	}
 }
 
 static int cgroup_show_options(struct seq_file *seq, struct kernfs_root *kf_root)
 {
-	if (cgrp_dfl_root.flags & CGRP_ROOT_NS_DELEGATE)
-		seq_puts(seq, ",nsdelegate");
-	if (cgrp_dfl_root.flags & CGRP_ROOT_FAVOR_DYNMODS)
-		seq_puts(seq, ",favordynmods");
-	if (cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_LOCAL_EVENTS)
-		seq_puts(seq, ",memory_localevents");
-	if (cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_RECURSIVE_PROT)
-		seq_puts(seq, ",memory_recursiveprot");
-	if (cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING)
-		seq_puts(seq, ",memory_hugetlb_accounting");
-	if (cgrp_dfl_root.flags & CGRP_ROOT_PIDS_LOCAL_EVENTS)
-		seq_puts(seq, ",pids_localevents");
+	int i;
+
+	for (i = 0; i < nr__cgroup2_params; ++i) {
+		if (cgrp_dfl_root.flags & mount_flags_desc[i].flag) {
+			seq_puts(seq, ",");
+			seq_puts(seq, mount_flags_desc[i].name);
+		}
+	}
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH -next] cgroup: Use descriptor table to unify mount flag management

[Chen Ridong]

On 2025/11/26 10:08, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
> 
> The cgroup2 mount flags (e.g. nsdelegate, favordynmods) were previously
> handled via scattered switch-case and conditional checks across
> parameter parsing, flag application, and option display paths. This
> leads to redundant code and increased maintenance cost when adding/removing
> flags.
> 
> Introduce a `cgroup_mount_flag_desc` descriptor table to centralize the
> mapping between flag bits, names, and apply functions. Refactor the
> relevant paths to use this table for unified management:
> 1. cgroup2_parse_param: Replace switch-case with table lookup
> 2. apply_cgroup_root_flags: Replace multiple conditionals with table
>    iteration
> 3. cgroup_show_options: Replace hardcoded seq_puts with table-driven output
> 
> No functional change intended, and the mount option output format remains
> compatible with the original implementation.
> 

Hi all,

Would anyone be interested?

-- 
Best regards,
Ridong

Re: [PATCH -next] cgroup: Use descriptor table to unify mount flag management

[Michal Koutný]

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

Hi Ridong.

On Wed, Nov 26, 2025 at 02:08:25AM +0000, Chen Ridong <chenridong@huaweicloud.com> wrote:
> From: Chen Ridong <chenridong@huawei.com>
> 
> The cgroup2 mount flags (e.g. nsdelegate, favordynmods) were previously
> handled via scattered switch-case and conditional checks across
> parameter parsing, flag application, and option display paths. This
> leads to redundant code and increased maintenance cost when adding/removing
> flags.
> 
> Introduce a `cgroup_mount_flag_desc` descriptor table to centralize the
> mapping between flag bits, names, and apply functions. Refactor the
> relevant paths to use this table for unified management:
> 1. cgroup2_parse_param: Replace switch-case with table lookup
> 2. apply_cgroup_root_flags: Replace multiple conditionals with table
>    iteration
> 3. cgroup_show_options: Replace hardcoded seq_puts with table-driven output
> 
> No functional change intended, and the mount option output format remains
> compatible with the original implementation.

At first I thought this is worthy but then I ran into the possible
(semantic) overlap with the cgroup2_fs_parameters array (the string
`name`s are duplicated in both :-/), I didn't figure out a way how to
make such an polymorphic array in C (like when cgroup_mount_flag_desc
would be a class that inherits from fs_parameter_spec and you could pass
the array of the formers to consumers (fs_parse()) of latters).

So I'm wondering whether there exists some way to avoid possible
divergence between definitions of the two arrays...

(Below are some notes I had made.)

> 
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>  kernel/cgroup/cgroup.c | 107 +++++++++++++++++++----------------------
>  1 file changed, 49 insertions(+), 58 deletions(-)
> 
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index e717208cfb18..1e4033d05c29 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -2005,6 +2005,36 @@ static const struct fs_parameter_spec cgroup2_fs_parameters[] = {
>  	{}
>  };
>  
> +struct cgroup_mount_flag_desc {
> +	u64 flag;

This could use an enum for CGROUP_ROOT_* values.
(For readability/convenience.)

> +	const char *name;
> +	void (*apply)(struct cgroup_root *root, u64 bit, bool enable);

I leave up to your discretion whether the cgroup_root should be passed
explicitly (it's always cgrp_dfl_root and existing functions reach to
the symbol already).

> +};
> +
> +static void apply_cgroup_favor_flags(struct cgroup_root *root,
> +					     u64 bit, bool enable)
> +{
> +	return cgroup_favor_dynmods(root, enable);
> +}
> +
> +static void __apply_cgroup_root_flags(struct cgroup_root *root,
> +					      u64 bit, bool enable)

I think double underscore is overkill given `static` and the previous
helper.

> +{
> +	if (enable)
> +		root->flags |= bit;
> +	else
> +		root->flags &= ~bit;
> +}
> +
> +static const struct cgroup_mount_flag_desc mount_flags_desc[nr__cgroup2_params] = {
> +{CGRP_ROOT_NS_DELEGATE, "nsdelegate", __apply_cgroup_root_flags},
> +{CGRP_ROOT_FAVOR_DYNMODS, "favordynmods", apply_cgroup_favor_flags},
> +{CGRP_ROOT_MEMORY_LOCAL_EVENTS, "memory_localevents", __apply_cgroup_root_flags},
> +{CGRP_ROOT_MEMORY_RECURSIVE_PROT, "memory_recursiveprot", __apply_cgroup_root_flags},
> +{CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING, "memory_hugetlb_accounting", __apply_cgroup_root_flags},
> +{CGRP_ROOT_PIDS_LOCAL_EVENTS, "pids_localevents", __apply_cgroup_root_flags}
> +};

It seems indentation is missing here.
This is actually a specialization of the cgroup2_fs_parameters array...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]

Re: [PATCH -next] cgroup: Use descriptor table to unify mount flag management

[Chen Ridong]

On 2025/12/2 17:24, Michal Koutný wrote:
> Hi Ridong.
> 
> On Wed, Nov 26, 2025 at 02:08:25AM +0000, Chen Ridong <chenridong@huaweicloud.com> wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> The cgroup2 mount flags (e.g. nsdelegate, favordynmods) were previously
>> handled via scattered switch-case and conditional checks across
>> parameter parsing, flag application, and option display paths. This
>> leads to redundant code and increased maintenance cost when adding/removing
>> flags.
>>
>> Introduce a `cgroup_mount_flag_desc` descriptor table to centralize the
>> mapping between flag bits, names, and apply functions. Refactor the
>> relevant paths to use this table for unified management:
>> 1. cgroup2_parse_param: Replace switch-case with table lookup
>> 2. apply_cgroup_root_flags: Replace multiple conditionals with table
>>    iteration
>> 3. cgroup_show_options: Replace hardcoded seq_puts with table-driven output
>>
>> No functional change intended, and the mount option output format remains
>> compatible with the original implementation.
> 
> At first I thought this is worthy but then I ran into the possible
> (semantic) overlap with the cgroup2_fs_parameters array (the string
> `name`s are duplicated in both :-/), I didn't figure out a way how to
> make such an polymorphic array in C (like when cgroup_mount_flag_desc
> would be a class that inherits from fs_parameter_spec and you could pass
> the array of the formers to consumers (fs_parse()) of latters).
> 
> So I'm wondering whether there exists some way to avoid possible
> divergence between definitions of the two arrays...
> 

Hi Michal,

Thank you for your thoughtful feedback.

I understand your concern about the semantic overlap between the two arrays and the potential for
divergence. I initially tried to find a way to merge them into a single polymorphic array, but given
the constraints of C and the existing fs_parameter_spec structure (which we cannot easily modify for
this purpose), I haven't found a clean way to achieve that.

However, to address the maintenance issue, I've come up with an alternative approach using a macro
that allows us to define mount flags in just one place. The idea is to introduce a macro list
CGROUP2_MOUNT_FLAG_LIST that expands into both the fs_parameter_spec array and the new
cgroup_mount_flag_desc table. This way, when adding a new mount flag, we only need to extend this
single macro list.

While we still end up with two separate arrays, the macro ensures that any addition or modification
only needs to be made in one place—the CGROUP2_MOUNT_FLAG_LIST. This should prevent the divergence
you mentioned.

What do you think about this approach? If you have any suggestions for further improvement, I'd be
happy to incorporate them.

Below is a simplified diff showing the concept:

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index e717208cfb18..bd81b15dc3bd 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1985,26 +1985,53 @@ int cgroup_show_path(struct seq_file *sf, struct kernfs_node *kf_node,
 	return len;
 }

+#define CGROUP2_MOUNT_FLAG_LIST(_)					\
+	_(nsdelegate,		CGRP_ROOT_NS_DELEGATE,	apply_cgroup_root_flag) \
+	_(favordynmods,	CGRP_ROOT_FAVOR_DYNMODS,	apply_cgroup_favor_flag) \
+	_(memory_localevents,	CGRP_ROOT_MEMORY_LOCAL_EVENTS, apply_cgroup_root_flag) \
+	_(memory_recursiveprot,	CGRP_ROOT_MEMORY_RECURSIVE_PROT, apply_cgroup_root_flag) \
+	_(memory_hugetlb_accounting, CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING, apply_cgroup_root_flag) \
+	_(pids_localevents,	CGRP_ROOT_PIDS_LOCAL_EVENTS, apply_cgroup_root_flag)
+
 enum cgroup2_param {
-	Opt_nsdelegate,
-	Opt_favordynmods,
-	Opt_memory_localevents,
-	Opt_memory_recursiveprot,
-	Opt_memory_hugetlb_accounting,
-	Opt_pids_localevents,
+#define CGROUP2_PARAM_ENUM(name, ...) Opt_##name,
+	CGROUP2_MOUNT_FLAG_LIST(CGROUP2_PARAM_ENUM)
+#undef CGROUP2_PARAM_ENUM
 	nr__cgroup2_params
 };

+struct cgroup_mount_flag_desc {
+	enum cgroup_root_flag flag;
+	const char *name;
+	void (*apply)(enum cgroup_root_flag flag, bool enable);
+};
+
+static void apply_cgroup_root_flag(enum cgroup_root_flag flag, bool enable)
+{
+	if (enable)
+		cgrp_dfl_root.flags |= flag;
+	else
+		cgrp_dfl_root.flags &= ~flag;
+}
+
+static void apply_cgroup_favor_flag(enum cgroup_root_flag flag, bool enable)
+{
+	cgroup_favor_dynmods(&cgrp_dfl_root, enable);
+}
+
 static const struct fs_parameter_spec cgroup2_fs_parameters[] = {
-	fsparam_flag("nsdelegate",		Opt_nsdelegate),
-	fsparam_flag("favordynmods",		Opt_favordynmods),
-	fsparam_flag("memory_localevents",	Opt_memory_localevents),
-	fsparam_flag("memory_recursiveprot",	Opt_memory_recursiveprot),
-	fsparam_flag("memory_hugetlb_accounting", Opt_memory_hugetlb_accounting),
-	fsparam_flag("pids_localevents",	Opt_pids_localevents),
+#define CGROUP2_PARAM_SPEC(name, ...) fsparam_flag(#name, Opt_##name),
+	CGROUP2_MOUNT_FLAG_LIST(CGROUP2_PARAM_SPEC)
+#undef CGROUP2_PARAM_SPEC
 	{}
 };

+static const struct cgroup_mount_flag_desc cgroup2_mount_flags[] = {
+#define CGROUP2_FLAG_DESC(name, flag, apply)[Opt_##name] = { flag, #name, apply },
+	CGROUP2_MOUNT_FLAG_LIST(CGROUP2_FLAG_DESC)
+#undef CGROUP2_FLAG_DESC
+};
+
...

-- 
Best regards,
Ridong

Re: [PATCH -next] cgroup: Use descriptor table to unify mount flag management

[Michal Koutný]

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

On Tue, Dec 02, 2025 at 09:53:11PM +0800, Chen Ridong <chenridong@huaweicloud.com> wrote:
> What do you think about this approach? If you have any suggestions for further improvement, I'd be
> happy to incorporate them.

Yes, it's better due to the single place of definition.
It made me to look around at some other filesystems from a random sample
(skewed towards ones with more options) and I see:
- many of them simply use the big switch/case in .parse_param,
- ext4 has its specialized ext4_mount_opts array whose order needn't
  match ext4_param_specs thanks to dynamic search.

All in all, I appreciate your effort, however, I'm not sure it's worth
to deviate from the custom of other FS implementations.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]

Re: [PATCH -next] cgroup: Use descriptor table to unify mount flag management

[Chen Ridong]

On 2025/12/5 0:23, Michal Koutný wrote:
> On Tue, Dec 02, 2025 at 09:53:11PM +0800, Chen Ridong <chenridong@huaweicloud.com> wrote:
>> What do you think about this approach? If you have any suggestions for further improvement, I'd be
>> happy to incorporate them.
> 
> Yes, it's better due to the single place of definition.
> It made me to look around at some other filesystems from a random sample
> (skewed towards ones with more options) and I see:
> - many of them simply use the big switch/case in .parse_param,
> - ext4 has its specialized ext4_mount_opts array whose order needn't
>   match ext4_param_specs thanks to dynamic search.
> 
> All in all, I appreciate your effort, however, I'm not sure it's worth
> to deviate from the custom of other FS implementations.
> 
> Michal

Thank you for your feedback.

I also examined other filesystems, and you are correct—most do use a large switch/case structure in
.parse_param. However, none seem to have to maintain logic across three different functions like
cgroup does.

My intention was to avoid further expanding the if/switch chains, but given how many options we
already handle, perhaps a refactor isn't immediately necessary. We can leave it as is for now. If
mount options continue to increase, we might reconsider refactoring in the future.

Thank you again.

-- 
Best regards,
Ridong