Re: [PATCH 1/4] cgroup: Eliminate cgrp_ancestor_storage in cgroup_root

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

On 12/18/25 01:27, Michal Koutný wrote:
> The cgrp_ancestor_storage has two drawbacks:
> - it's not guaranteed that the member immediately follows struct cgrp in
>    cgroup_root (root cgroup's ancestors[0] might thus point to a padding
>    and not in cgrp_ancestor_storage proper),
> - this idiom raises warnings with -Wflex-array-member-not-at-end.
> 
> Instead of relying on the auxiliary member in cgroup_root, define the
> 0-th level ancestor inside struct cgroup (needed for static allocation
> of cgrp_dfl_root), deeper cgroups would allocate flexible
> _low_ancestors[].  Unionized alias through ancestors[] will
> transparently join the two ranges (ancestors is wrapped in a struct to
> avoid 'error: flexible array member in union').
> 
> The above change would still leave the flexible array at the end of
> struct cgroup, so move cgrp also towards the end of cgroup_root to
> resolve the -Wflex-array-member-not-at-end.
> 
> Link: https://lore.kernel.org/r/5fb74444-2fbb-476e-b1bf-3f3e279d0ced@embeddedor.com/
> Reported-by: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
> Closes: https://lore.kernel.org/r/b3eb050d-9451-4b60-b06c-ace7dab57497@embeddedor.com/
> Cc: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
> ---
>   include/linux/cgroup-defs.h | 28 +++++++++++++++++-----------
>   kernel/cgroup/cgroup.c      |  2 +-
>   2 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
> index b760a3c470a56..9247e437da5ce 100644
> --- a/include/linux/cgroup-defs.h
> +++ b/include/linux/cgroup-defs.h
> @@ -626,7 +626,16 @@ struct cgroup {
>   #endif
>   
>   	/* All ancestors including self */
> -	struct cgroup *ancestors[];
> +	union {
> +		struct {
> +			void *_sentinel[0]; /* XXX to avoid 'flexible array member in a struct with no named members' */
> +			struct cgroup *ancestors[];
> +		};

Instead of the above anonymous struct, we can use the DECLARE_FLEX_ARRAY()
helper here:

		DECLARE_FLEX_ARRAY(struct cgroup, *ancestors);

In any case:

Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
-Gustavo

> +		struct {
> +			struct cgroup *_root_ancestor;
> +			struct cgroup *_low_ancestors[];
> +		};
> +	};
>   };
>   
>   /*
> @@ -647,16 +656,6 @@ struct cgroup_root {
>   	struct list_head root_list;
>   	struct rcu_head rcu;	/* Must be near the top */
>   
> -	/*
> -	 * The root cgroup. The containing cgroup_root will be destroyed on its
> -	 * release. cgrp->ancestors[0] will be used overflowing into the
> -	 * following field. cgrp_ancestor_storage must immediately follow.
> -	 */
> -	struct cgroup cgrp;
> -
> -	/* must follow cgrp for cgrp->ancestors[0], see above */
> -	struct cgroup *cgrp_ancestor_storage;
> -
>   	/* Number of cgroups in the hierarchy, used only for /proc/cgroups */
>   	atomic_t nr_cgrps;
>   
> @@ -668,6 +667,13 @@ struct cgroup_root {
>   
>   	/* The name for this hierarchy - may be empty */
>   	char name[MAX_CGROUP_ROOT_NAMELEN];
> +
> +	/*
> +	 * The root cgroup. The containing cgroup_root will be destroyed on its
> +	 * release. This must be embedded last due to flexible array at the end
> +	 * of struct cgroup.
> +	 */
> +	struct cgroup cgrp;
>   };
>   
>   /*
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index e717208cfb185..554a02ee298ba 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -5847,7 +5847,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name,
>   	int ret;
>   
>   	/* allocate the cgroup and its ID, 0 is reserved for the root */
> -	cgrp = kzalloc(struct_size(cgrp, ancestors, (level + 1)), GFP_KERNEL);
> +	cgrp = kzalloc(struct_size(cgrp, _low_ancestors, level), GFP_KERNEL);
>   	if (!cgrp)
>   		return ERR_PTR(-ENOMEM);
>