Re: [RFC] cgroup: Avoid thousands of -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

On 9/1/25 19:58, Michal Koutný wrote:

On Mon, Sep 01, 2025 at 05:21:22PM +0200, "Gustavo A. R. Silva" 
<gust...@embeddedor.com> wrote:

Because struct cgroup ends in a flexible-array member `ancestors`.
This triggers the -Wflex-array-member-not-at-end warns about. So,
while `ancestors` is indeed a flexible array, any instance of
cgroup embedded in another struct should be placed at the end.

Oh, so TRAILING_OVERLAP() won't work like that?
(I thought that it'd hide the FAM from the end of the union and thus it
could embedded when wrapped like this. On second thought, I realize
that's exclusive with the static validations.)

However, if we change it to something like this (and of course
updating any related code, accordingly):

-       struct cgroup *ancestors[];
+       struct cgroup **ancestors;

Then the flex in the middle issue goes away, and we can have
struct cgroup embedded in another struct anywhere.

The question is if this would be an acceptable solution?

I'd probably prefer this to remain a flexible-array member,
but I'd like to hear people's opinions and feedback. :)

I'd prefer if cgroup_create could still work with one allocation only
both for struct cgroup and its ancestors array. (Cgroup allocation
happens many times in a day.)

The increase in struct cgroup_root size is IMO not that problematic.
(There are typically at most CGROUP_SUBSYS_COUNT roots with gradual
trend to only the single cgrp_dfl_root.)

Note that it'd be good to keep it enclosed within struct cgroup_root
(cgroup1_root_to_use could use struct_size()), however, the
cgrp_dfl_root would still need the storage somewhere.

If the increase in size is not a problem, then something like this
works fine (unless there is a problem with moving those two members
at the end of cgroup_root?):

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 539c64eeef38..bd28d639a78a 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -630,16 +630,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;

@@ -651,7 +641,21 @@ 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. cgrp->ancestors[0] will be used overflowing into the
+        * following field. cgrp_ancestor_storage must immediately follow.
+        *
+        * Must be last --ends in a flexible-array member.
+        */
+       TRAILING_OVERLAP(struct cgroup, cgrp, ancestors,
+               /* must follow cgrp for cgrp->ancestors[0], see above */
+               struct cgroup *cgrp_ancestor_storage;
+       );
 };
+static_assert(offsetof(struct cgroup_root, cgrp.ancestors) ==
+             offsetof(struct cgroup_root, cgrp_ancestor_storage));


The assert above checks that no misalignment is inadvertently
introduced between FAM and cgrp_ancestor_storage.

Thanks
-Gustavo