[PATCH 0/4] Use __counted_by for ancestor arrays

[PATCH 0/4] Use __counted_by for ancestor arrays

[Michal Koutný]

The trick with utilizing space in cgroup_root for cgroup::ancetors flex
array was an obstacle for kernel reworks for
-Wflex-array-member-not-at-end.

The first patch fixes that, then I wanted to utilize __counted_by for
this flex array which required some more rework how cgroup level is
evaluated.

Similar flex array is also in struct ioc_gq where it was tempting to
simply use __counted_by(level), however, this would be off-by-one as it
has semantics like cgroup's level (0 == root).
Proper adjustment for __counted_by() would either need similar
level/ancestor helpers or abstracted macros for ancestors up/down
iterations.

I only made a simple comment fixup since I'm not sure about benefit of
__counted_by for structs that aren't sized based on direct user input.

Michal Koutný (4):
  cgroup: Eliminate cgrp_ancestor_storage in cgroup_root
  cgroup: Introduce cgroup_level() helper
  cgroup: Use __counted_by for cgroup::ancestors
  blk-iocost: Correct comment ioc_gq::level

 block/bfq-iosched.c           |  2 +-
 block/blk-iocost.c            |  6 ++---
 include/linux/cgroup-defs.h   | 43 +++++++++++++++++++----------------
 include/linux/cgroup.h        | 18 ++++++++++++---
 include/trace/events/cgroup.h |  8 +++----
 kernel/bpf/helpers.c          |  2 +-
 kernel/cgroup/cgroup.c        |  9 ++++----
 net/netfilter/nft_socket.c    |  2 +-
 8 files changed, 53 insertions(+), 37 deletions(-)


base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
-- 
2.52.0

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

[Michal Koutný]

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[];
+		};
+		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);
 
-- 
2.52.0

[PATCH 2/4] cgroup: Introduce cgroup_level() helper

[Michal Koutný]

This is a no functional change to hide physical storage of cgroup's
level and it allows subesequent conversion of the storage.

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 block/bfq-iosched.c           |  2 +-
 block/blk-iocost.c            |  4 ++--
 include/linux/cgroup.h        | 18 +++++++++++++++---
 include/trace/events/cgroup.h |  8 ++++----
 kernel/bpf/helpers.c          |  2 +-
 kernel/cgroup/cgroup.c        |  4 ++--
 net/netfilter/nft_socket.c    |  2 +-
 7 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 4a8d3d96bfe49..f293bab068274 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -601,7 +601,7 @@ static bool bfqq_request_over_limit(struct bfq_data *bfqd,
 		goto out;
 
 	/* +1 for bfqq entity, root cgroup not included */
-	depth = bfqg_to_blkg(bfqq_group(bfqq))->blkcg->css.cgroup->level + 1;
+	depth = cgroup_level(bfqg_to_blkg(bfqq_group(bfqq))->blkcg->css.cgroup) + 1;
 	if (depth > alloc_depth) {
 		spin_unlock_irq(&bfqd->lock);
 		if (entities != inline_entities)
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index a0416927d33dc..b4eebe61dca7f 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -2962,7 +2962,7 @@ static void ioc_cpd_free(struct blkcg_policy_data *cpd)
 static struct blkg_policy_data *ioc_pd_alloc(struct gendisk *disk,
 		struct blkcg *blkcg, gfp_t gfp)
 {
-	int levels = blkcg->css.cgroup->level + 1;
+	int levels = cgroup_level(blkcg->css.cgroup) + 1;
 	struct ioc_gq *iocg;
 
 	iocg = kzalloc_node(struct_size(iocg, ancestors, levels), gfp,
@@ -3003,7 +3003,7 @@ static void ioc_pd_init(struct blkg_policy_data *pd)
 	init_waitqueue_head(&iocg->waitq);
 	hrtimer_setup(&iocg->waitq_timer, iocg_waitq_timer_fn, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
 
-	iocg->level = blkg->blkcg->css.cgroup->level;
+	iocg->level = cgroup_level(blkg->blkcg->css.cgroup)
 
 	for (tblkg = blkg; tblkg; tblkg = tblkg->parent) {
 		struct ioc_gq *tiocg = blkg_to_iocg(tblkg);
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index bc892e3b37eea..0290878ebad26 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -525,6 +525,18 @@ static inline struct cgroup *cgroup_parent(struct cgroup *cgrp)
 	return NULL;
 }
 
+/**
+ * cgroup_level - cgroup depth
+ * @cgrp: cgroup
+ *
+ * The depth this cgroup is at.  The root is at depth zero and each step down
+ * the hierarchy increments the level.
+ */
+static inline int cgroup_level(struct cgroup *cgrp)
+{
+	return cgrp->level;
+}
+
 /**
  * cgroup_is_descendant - test ancestry
  * @cgrp: the cgroup to be tested
@@ -537,9 +549,9 @@ static inline struct cgroup *cgroup_parent(struct cgroup *cgrp)
 static inline bool cgroup_is_descendant(struct cgroup *cgrp,
 					struct cgroup *ancestor)
 {
-	if (cgrp->root != ancestor->root || cgrp->level < ancestor->level)
+	if (cgrp->root != ancestor->root || cgroup_level(cgrp) < cgroup_level(ancestor))
 		return false;
-	return cgrp->ancestors[ancestor->level] == ancestor;
+	return cgrp->ancestors[cgroup_level(ancestor)] == ancestor;
 }
 
 /**
@@ -556,7 +568,7 @@ static inline bool cgroup_is_descendant(struct cgroup *cgrp,
 static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
 					     int ancestor_level)
 {
-	if (ancestor_level < 0 || ancestor_level > cgrp->level)
+	if (ancestor_level < 0 || ancestor_level > cgroup_level(cgrp))
 		return NULL;
 	return cgrp->ancestors[ancestor_level];
 }
diff --git a/include/trace/events/cgroup.h b/include/trace/events/cgroup.h
index ba9229af9a343..0a1bc91754b5e 100644
--- a/include/trace/events/cgroup.h
+++ b/include/trace/events/cgroup.h
@@ -67,7 +67,7 @@ DECLARE_EVENT_CLASS(cgroup,
 	TP_fast_assign(
 		__entry->root = cgrp->root->hierarchy_id;
 		__entry->id = cgroup_id(cgrp);
-		__entry->level = cgrp->level;
+		__entry->level = cgroup_level(cgrp);
 		__assign_str(path);
 	),
 
@@ -136,7 +136,7 @@ DECLARE_EVENT_CLASS(cgroup_migrate,
 	TP_fast_assign(
 		__entry->dst_root = dst_cgrp->root->hierarchy_id;
 		__entry->dst_id = cgroup_id(dst_cgrp);
-		__entry->dst_level = dst_cgrp->level;
+		__entry->dst_level = cgroup_level(dst_cgrp);
 		__assign_str(dst_path);
 		__entry->pid = task->pid;
 		__assign_str(comm);
@@ -180,7 +180,7 @@ DECLARE_EVENT_CLASS(cgroup_event,
 	TP_fast_assign(
 		__entry->root = cgrp->root->hierarchy_id;
 		__entry->id = cgroup_id(cgrp);
-		__entry->level = cgrp->level;
+		__entry->level = cgroup_level(cgrp);
 		__assign_str(path);
 		__entry->val = val;
 	),
@@ -221,7 +221,7 @@ DECLARE_EVENT_CLASS(cgroup_rstat,
 	TP_fast_assign(
 		__entry->root = cgrp->root->hierarchy_id;
 		__entry->id = cgroup_id(cgrp);
-		__entry->level = cgrp->level;
+		__entry->level = cgroup_level(cgrp);
 		__entry->cpu = cpu;
 		__entry->contended = contended;
 	),
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index db72b96f9c8c8..b825f6e0a1c29 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2577,7 +2577,7 @@ __bpf_kfunc struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level)
 {
 	struct cgroup *ancestor;
 
-	if (level > cgrp->level || level < 0)
+	if (level > cgroup_level(cgrp) || level < 0)
 		return NULL;
 
 	/* cgrp's refcnt could be 0 here, but ancestors can still be accessed */
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 554a02ee298ba..e011f1dd6d87f 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5843,7 +5843,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name,
 	struct cgroup_root *root = parent->root;
 	struct cgroup *cgrp, *tcgrp;
 	struct kernfs_node *kn;
-	int i, level = parent->level + 1;
+	int i, level = cgroup_level(parent) + 1;
 	int ret;
 
 	/* allocate the cgroup and its ID, 0 is reserved for the root */
@@ -5884,7 +5884,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name,
 		goto out_stat_exit;
 
 	for (tcgrp = cgrp; tcgrp; tcgrp = cgroup_parent(tcgrp))
-		cgrp->ancestors[tcgrp->level] = tcgrp;
+		cgrp->ancestors[cgroup_level(tcgrp)] = tcgrp;
 
 	/*
 	 * New cgroup inherits effective freeze counter, and
diff --git a/net/netfilter/nft_socket.c b/net/netfilter/nft_socket.c
index 36affbb697c2f..a5b0340924efb 100644
--- a/net/netfilter/nft_socket.c
+++ b/net/netfilter/nft_socket.c
@@ -64,7 +64,7 @@ static noinline int nft_socket_cgroup_subtree_level(void)
 	if (IS_ERR(cgrp))
 		return PTR_ERR(cgrp);
 
-	level = cgrp->level;
+	level = cgroup_level(cgrp);
 
 	cgroup_put(cgrp);
 
-- 
2.52.0

[PATCH 3/4] cgroup: Use __counted_by for cgroup::ancestors

[Michal Koutný]

cgroup::ancestors includes self, i.e. root cgroups have one ancestor but
their level is 0. Change the value that we store inside struct cgroup
and use an inlined helper where we need to know the level. This way we
preserve the concept of 0-based levels and we can utilize __counted_by
constraint to guard ancestors access. (We could've used level value as a
counter for _low_ancestors but that would have no benefit since we never
access data through this flexible array alias.)

Cc: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 include/linux/cgroup-defs.h | 19 ++++++++-----------
 include/linux/cgroup.h      |  2 +-
 kernel/cgroup/cgroup.c      |  3 ++-
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 9247e437da5ce..8ce1ae9bea909 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -475,14 +475,6 @@ struct cgroup {
 
 	unsigned long flags;		/* "unsigned long" so bitops work */
 
-	/*
-	 * The depth this cgroup is at.  The root is at depth zero and each
-	 * step down the hierarchy increments the level.  This along with
-	 * ancestors[] can determine whether a given cgroup is a
-	 * descendant of another without traversing the hierarchy.
-	 */
-	int level;
-
 	/* Maximum allowed descent tree depth */
 	int max_depth;
 
@@ -625,13 +617,18 @@ struct cgroup {
 	struct bpf_local_storage __rcu  *bpf_cgrp_storage;
 #endif
 
-	/* All ancestors including self */
 	union {
 		struct {
-			void *_sentinel[0]; /* XXX to avoid 'flexible array member in a struct with no named members' */
-			struct cgroup *ancestors[];
+			int nr_ancestors;	/* do not use directly but via cgroup_level() */
+			/*
+			 * All ancestors including self.
+			 * ancestors[] can determine whether a given cgroup is a
+			 * descendant of another without traversing the hierarchy.
+			 */
+			struct cgroup *ancestors[] __counted_by(nr_ancestors);
 		};
 		struct {
+			int _nr_ancestors;	/* auxiliary padding, see nr_ancestors above */
 			struct cgroup *_root_ancestor;
 			struct cgroup *_low_ancestors[];
 		};
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 0290878ebad26..45f720b9ecedd 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -534,7 +534,7 @@ static inline struct cgroup *cgroup_parent(struct cgroup *cgrp)
  */
 static inline int cgroup_level(struct cgroup *cgrp)
 {
-	return cgrp->level;
+	return cgrp->nr_ancestors - 1;
 }
 
 /**
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index e011f1dd6d87f..5110d3e13d125 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2197,6 +2197,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
 	}
 	root_cgrp->kn = kernfs_root_to_node(root->kf_root);
 	WARN_ON_ONCE(cgroup_ino(root_cgrp) != 1);
+	root_cgrp->nr_ancestors = 1; /* stored in _root_ancestor */
 	root_cgrp->ancestors[0] = root_cgrp;
 
 	ret = css_populate_dir(&root_cgrp->self);
@@ -5869,7 +5870,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name,
 
 	cgrp->self.parent = &parent->self;
 	cgrp->root = root;
-	cgrp->level = level;
+	cgrp->nr_ancestors = parent->nr_ancestors + 1;
 
 	/*
 	 * Now that init_cgroup_housekeeping() has been called and cgrp->self
-- 
2.52.0

[PATCH 4/4] blk-iocost: Correct comment ioc_gq::level

[Michal Koutný]

This comment is simpler than reworking level users for possible
ioc_gq::ancestors __counted_by annotation.

Cc: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 block/blk-iocost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index b4eebe61dca7f..c5e09ebae5ab0 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -545,7 +545,7 @@ struct ioc_gq {
 	u64				indebt_since;
 	u64				indelay_since;
 
-	/* this iocg's depth in the hierarchy and ancestors including self */
+	/* this iocg's depth in the hierarchy and ancestors excluding self */
 	int				level;
 	struct ioc_gq			*ancestors[];
 };
-- 
2.52.0

Re: [PATCH 2/4] cgroup: Introduce cgroup_level() helper

[bot+bpf-ci]

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

> diff --git a/block/blk-iocost.c b/block/blk-iocost.c
> index a0416927d33d..b4eebe61dca7 100644
> --- a/block/blk-iocost.c
> +++ b/block/blk-iocost.c

[ ... ]

> @@ -3003,7 +3003,7 @@ static void ioc_pd_init(struct blkg_policy_data *pd)
>  	init_waitqueue_head(&iocg->waitq);
>  	hrtimer_setup(&iocg->waitq_timer, iocg_waitq_timer_fn, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
>
> -	iocg->level = blkg->blkcg->css.cgroup->level;
> +	iocg->level = cgroup_level(blkg->blkcg->css.cgroup)
                                                              ^
Is this missing a semicolon? The line should end with a semicolon to
make this valid C syntax. This will prevent the kernel from building
in the ioc_pd_init() function.

>
>  	for (tblkg = blkg; tblkg; tblkg = tblkg->parent) {
>  		struct ioc_gq *tiocg = blkg_to_iocg(tblkg);


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20310204023

Re: [PATCH 4/4] blk-iocost: Correct comment ioc_gq::level

[Tejun Heo]

On Wed, Dec 17, 2025 at 05:27:36PM +0100, Michal Koutný wrote:
> This comment is simpler than reworking level users for possible
> ioc_gq::ancestors __counted_by annotation.

I don't understand the change here. Can you please elaborate a bit more?

Thanks.

-- 
tejun

Re: [PATCH 4/4] blk-iocost: Correct comment ioc_gq::level

[Michal Koutný]

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

On Wed, Dec 17, 2025 at 06:57:05AM -1000, Tejun Heo <tj@kernel.org> wrote:
> On Wed, Dec 17, 2025 at 05:27:36PM +0100, Michal Koutný wrote:
> > This comment is simpler than reworking level users for possible
> > ioc_gq::ancestors __counted_by annotation.
> 
> I don't understand the change here. Can you please elaborate a bit more?

ioc_gq::ancestors includes self but ioc_gq::level doesn't count it in
(level=0 is root, that's like cgroup's level, from which it's copied in
ioc_pd_init()). Therefore ioc_gq::level cannot be used as size hint of
the ancestors array :-/ (The comment in the original form tempted to
simply use __counted_by(level). I see a comment for each member would be
the clearest.)

I'm open to more remarks or questions.

Michal

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

Re: [PATCH 3/4] cgroup: Use __counted_by for cgroup::ancestors

[Chen Ridong]

On 2025/12/18 0:27, Michal Koutný wrote:
> cgroup::ancestors includes self, i.e. root cgroups have one ancestor but
> their level is 0. Change the value that we store inside struct cgroup
> and use an inlined helper where we need to know the level. This way we
> preserve the concept of 0-based levels and we can utilize __counted_by
> constraint to guard ancestors access. (We could've used level value as a
> counter for _low_ancestors but that would have no benefit since we never
> access data through this flexible array alias.)
> 
> Cc: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
> ---
>  include/linux/cgroup-defs.h | 19 ++++++++-----------
>  include/linux/cgroup.h      |  2 +-
>  kernel/cgroup/cgroup.c      |  3 ++-
>  3 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
> index 9247e437da5ce..8ce1ae9bea909 100644
> --- a/include/linux/cgroup-defs.h
> +++ b/include/linux/cgroup-defs.h
> @@ -475,14 +475,6 @@ struct cgroup {
>  
>  	unsigned long flags;		/* "unsigned long" so bitops work */
>  
> -	/*
> -	 * The depth this cgroup is at.  The root is at depth zero and each
> -	 * step down the hierarchy increments the level.  This along with
> -	 * ancestors[] can determine whether a given cgroup is a
> -	 * descendant of another without traversing the hierarchy.
> -	 */
> -	int level;
> -

Note that this level may already be used in existing BPF programs (e.g.,
tools/testing/selftests/bpf/progs/task_ls_uptr.c). Do we need to consider compatibility here?

>  	/* Maximum allowed descent tree depth */
>  	int max_depth;
>  
> @@ -625,13 +617,18 @@ struct cgroup {
>  	struct bpf_local_storage __rcu  *bpf_cgrp_storage;
>  #endif
>  
> -	/* All ancestors including self */
>  	union {
>  		struct {
> -			void *_sentinel[0]; /* XXX to avoid 'flexible array member in a struct with no named members' */
> -			struct cgroup *ancestors[];
> +			int nr_ancestors;	/* do not use directly but via cgroup_level() */
> +			/*
> +			 * All ancestors including self.
> +			 * ancestors[] can determine whether a given cgroup is a
> +			 * descendant of another without traversing the hierarchy.
> +			 */
> +			struct cgroup *ancestors[] __counted_by(nr_ancestors);
>  		};
>  		struct {
> +			int _nr_ancestors;	/* auxiliary padding, see nr_ancestors above */
>  			struct cgroup *_root_ancestor;
>  			struct cgroup *_low_ancestors[];
>  		};
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 0290878ebad26..45f720b9ecedd 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -534,7 +534,7 @@ static inline struct cgroup *cgroup_parent(struct cgroup *cgrp)
>   */
>  static inline int cgroup_level(struct cgroup *cgrp)
>  {
> -	return cgrp->level;
> +	return cgrp->nr_ancestors - 1;
>  }
>  
>  /**
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index e011f1dd6d87f..5110d3e13d125 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -2197,6 +2197,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
>  	}
>  	root_cgrp->kn = kernfs_root_to_node(root->kf_root);
>  	WARN_ON_ONCE(cgroup_ino(root_cgrp) != 1);
> +	root_cgrp->nr_ancestors = 1; /* stored in _root_ancestor */
>  	root_cgrp->ancestors[0] = root_cgrp;
>  
>  	ret = css_populate_dir(&root_cgrp->self);
> @@ -5869,7 +5870,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name,
>  
>  	cgrp->self.parent = &parent->self;
>  	cgrp->root = root;
> -	cgrp->level = level;
> +	cgrp->nr_ancestors = parent->nr_ancestors + 1;
>  
>  	/*
>  	 * Now that init_cgroup_housekeeping() has been called and cgrp->self

-- 
Best regards,
Ridong

Re: [PATCH 3/4] cgroup: Use __counted_by for cgroup::ancestors

[Tejun Heo]

On Thu, Dec 18, 2025 at 03:09:32PM +0800, Chen Ridong wrote:
> Note that this level may already be used in existing BPF programs (e.g.,
> tools/testing/selftests/bpf/progs/task_ls_uptr.c). Do we need to consider compatibility here?

That's a good point. Is __counted_by instrumentation tied to some compiler
flag? If so, might as well make it an optional extra field specifically for
the annotation rather than changing the meaning of an existing field.

Thanks.

-- 
tejun

Re: [PATCH 3/4] cgroup: Use __counted_by for cgroup::ancestors

[Michal Koutný]

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

On Thu, Dec 18, 2025 at 06:09:42AM -1000, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Dec 18, 2025 at 03:09:32PM +0800, Chen Ridong wrote:
> > Note that this level may already be used in existing BPF programs (e.g.,
> > tools/testing/selftests/bpf/progs/task_ls_uptr.c). Do we need to consider compatibility here?
> 
> That's a good point.

I wouldn't be concerned about this particular aspect. The commit
e6ac2450d6dee ("bpf: Support bpf program calling kernel function")
excludes ABIs, the example program uses ksyms (not kfuncs), so there
could even apply Documentation/process/stable-api-nonsense.rst.
OTOH, the semantics of level is unchanged for BPF helpers (that are the
official API).


> Is __counted_by instrumentation tied to some compiler flag? If so,
> might as well make it an optional extra field specifically for the
> annotation rather than changing the meaning of an existing field.

Honestly, I can see benefit mainly in the first patch of the series
(posted the rest for discussion).

I'd like to ask Gustavo whether __counted_by here buys us anything or
whether it's more useful in other parts of kernel (e.g. flexible
allocations in networking code with outer sources of data).

Thanks,
Michal

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

Re: [PATCH 3/4] cgroup: Use __counted_by for cgroup::ancestors

[Kees Cook]

On December 19, 2025 1:09:42 AM GMT+09:00, Tejun Heo <tj@kernel.org> wrote:
>On Thu, Dec 18, 2025 at 03:09:32PM +0800, Chen Ridong wrote:
>> Note that this level may already be used in existing BPF programs (e.g.,
>> tools/testing/selftests/bpf/progs/task_ls_uptr.c). Do we need to consider compatibility here?
>
>That's a good point. Is __counted_by instrumentation tied to some compiler
>flag? If so, might as well make it an optional extra field specifically for
>the annotation rather than changing the meaning of an existing field.
>
>Thanks.
>

CONFIG_FORTIFY_SOURCE and CONFIG_UBSAN_BOUNDS use the information for instrumentation.

-- 
Kees Cook

Re: [PATCH 2/4] cgroup: Introduce cgroup_level() helper

[kernel test robot]

Hi Michal,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8f0b4cce4481fb22653697cced8d0d04027cb1e8]

url:    https://github.com/intel-lab-lkp/linux/commits/Michal-Koutn/cgroup-Eliminate-cgrp_ancestor_storage-in-cgroup_root/20251218-004346
base:   8f0b4cce4481fb22653697cced8d0d04027cb1e8
patch link:    https://lore.kernel.org/r/20251217162744.352391-3-mkoutny%40suse.com
patch subject: [PATCH 2/4] cgroup: Introduce cgroup_level() helper
config: sparc64-randconfig-r134-20251218 (https://download.01.org/0day-ci/archive/20251220/202512202230.1uoB5chV-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251220/202512202230.1uoB5chV-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512202230.1uoB5chV-lkp@intel.com/

All errors (new ones prefixed by >>):

>> block/blk-iocost.c:3006:53: error: expected ';' after expression
    3006 |         iocg->level = cgroup_level(blkg->blkcg->css.cgroup)
         |                                                            ^
         |                                                            ;
   1 error generated.


vim +3006 block/blk-iocost.c

  2981	
  2982	static void ioc_pd_init(struct blkg_policy_data *pd)
  2983	{
  2984		struct ioc_gq *iocg = pd_to_iocg(pd);
  2985		struct blkcg_gq *blkg = pd_to_blkg(&iocg->pd);
  2986		struct ioc *ioc = q_to_ioc(blkg->q);
  2987		struct ioc_now now;
  2988		struct blkcg_gq *tblkg;
  2989		unsigned long flags;
  2990	
  2991		ioc_now(ioc, &now);
  2992	
  2993		iocg->ioc = ioc;
  2994		atomic64_set(&iocg->vtime, now.vnow);
  2995		atomic64_set(&iocg->done_vtime, now.vnow);
  2996		atomic64_set(&iocg->active_period, atomic64_read(&ioc->cur_period));
  2997		INIT_LIST_HEAD(&iocg->active_list);
  2998		INIT_LIST_HEAD(&iocg->walk_list);
  2999		INIT_LIST_HEAD(&iocg->surplus_list);
  3000		iocg->hweight_active = WEIGHT_ONE;
  3001		iocg->hweight_inuse = WEIGHT_ONE;
  3002	
  3003		init_waitqueue_head(&iocg->waitq);
  3004		hrtimer_setup(&iocg->waitq_timer, iocg_waitq_timer_fn, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
  3005	
> 3006		iocg->level = cgroup_level(blkg->blkcg->css.cgroup)
  3007	
  3008		for (tblkg = blkg; tblkg; tblkg = tblkg->parent) {
  3009			struct ioc_gq *tiocg = blkg_to_iocg(tblkg);
  3010			iocg->ancestors[tiocg->level] = tiocg;
  3011		}
  3012	
  3013		spin_lock_irqsave(&ioc->lock, flags);
  3014		weight_updated(iocg, &now);
  3015		spin_unlock_irqrestore(&ioc->lock, flags);
  3016	}
  3017	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/4] cgroup: Introduce cgroup_level() helper

[kernel test robot]

Hi Michal,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8f0b4cce4481fb22653697cced8d0d04027cb1e8]

url:    https://github.com/intel-lab-lkp/linux/commits/Michal-Koutn/cgroup-Eliminate-cgrp_ancestor_storage-in-cgroup_root/20251218-004346
base:   8f0b4cce4481fb22653697cced8d0d04027cb1e8
patch link:    https://lore.kernel.org/r/20251217162744.352391-3-mkoutny%40suse.com
patch subject: [PATCH 2/4] cgroup: Introduce cgroup_level() helper
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20251221/202512210532.ziNaxDJf-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251221/202512210532.ziNaxDJf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512210532.ziNaxDJf-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   block/blk-iocost.c: In function 'ioc_pd_init':
>> block/blk-iocost.c:3006:60: error: expected ';' before 'for'
    3006 |         iocg->level = cgroup_level(blkg->blkcg->css.cgroup)
         |                                                            ^
         |                                                            ;
    3007 | 
    3008 |         for (tblkg = blkg; tblkg; tblkg = tblkg->parent) {
         |         ~~~                                                 
>> block/blk-iocost.c:2988:26: warning: unused variable 'tblkg' [-Wunused-variable]
    2988 |         struct blkcg_gq *tblkg;
         |                          ^~~~~


vim +3006 block/blk-iocost.c

  2981	
  2982	static void ioc_pd_init(struct blkg_policy_data *pd)
  2983	{
  2984		struct ioc_gq *iocg = pd_to_iocg(pd);
  2985		struct blkcg_gq *blkg = pd_to_blkg(&iocg->pd);
  2986		struct ioc *ioc = q_to_ioc(blkg->q);
  2987		struct ioc_now now;
> 2988		struct blkcg_gq *tblkg;
  2989		unsigned long flags;
  2990	
  2991		ioc_now(ioc, &now);
  2992	
  2993		iocg->ioc = ioc;
  2994		atomic64_set(&iocg->vtime, now.vnow);
  2995		atomic64_set(&iocg->done_vtime, now.vnow);
  2996		atomic64_set(&iocg->active_period, atomic64_read(&ioc->cur_period));
  2997		INIT_LIST_HEAD(&iocg->active_list);
  2998		INIT_LIST_HEAD(&iocg->walk_list);
  2999		INIT_LIST_HEAD(&iocg->surplus_list);
  3000		iocg->hweight_active = WEIGHT_ONE;
  3001		iocg->hweight_inuse = WEIGHT_ONE;
  3002	
  3003		init_waitqueue_head(&iocg->waitq);
  3004		hrtimer_setup(&iocg->waitq_timer, iocg_waitq_timer_fn, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
  3005	
> 3006		iocg->level = cgroup_level(blkg->blkcg->css.cgroup)
  3007	
  3008		for (tblkg = blkg; tblkg; tblkg = tblkg->parent) {
  3009			struct ioc_gq *tiocg = blkg_to_iocg(tblkg);
  3010			iocg->ancestors[tiocg->level] = tiocg;
  3011		}
  3012	
  3013		spin_lock_irqsave(&ioc->lock, flags);
  3014		weight_updated(iocg, &now);
  3015		spin_unlock_irqrestore(&ioc->lock, flags);
  3016	}
  3017	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/4] cgroup: Use __counted_by for cgroup::ancestors

[Gustavo A. R. Silva]

On 12/19/25 01:32, Michal Koutný wrote:
> On Thu, Dec 18, 2025 at 06:09:42AM -1000, Tejun Heo <tj@kernel.org> wrote:
>> On Thu, Dec 18, 2025 at 03:09:32PM +0800, Chen Ridong wrote:
>>> Note that this level may already be used in existing BPF programs (e.g.,
>>> tools/testing/selftests/bpf/progs/task_ls_uptr.c). Do we need to consider compatibility here?
>>
>> That's a good point.
> 
> I wouldn't be concerned about this particular aspect. The commit
> e6ac2450d6dee ("bpf: Support bpf program calling kernel function")
> excludes ABIs, the example program uses ksyms (not kfuncs), so there
> could even apply Documentation/process/stable-api-nonsense.rst.
> OTOH, the semantics of level is unchanged for BPF helpers (that are the
> official API).
> 
> 
>> Is __counted_by instrumentation tied to some compiler flag? If so,
>> might as well make it an optional extra field specifically for the
>> annotation rather than changing the meaning of an existing field.
> 
> Honestly, I can see benefit mainly in the first patch of the series
> (posted the rest for discussion).
> 
> I'd like to ask Gustavo whether __counted_by here buys us anything or
> whether it's more useful in other parts of kernel (e.g. flexible
> allocations in networking code with outer sources of data).

Ideally, all structures containing a flexible-array member (FAM) should
be annotated. However, if this is too much of a hassle right now, I'd
say the priority is to avoid the -Wflex-array-member-not-at-end warnings,
first.

Thanks
-Gustavo

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

[Gustavo A. R. Silva]

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);
>   

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

[Tejun Heo]

On Tue, Jan 06, 2026 at 04:06:47PM +0900, Gustavo A. R. Silva wrote:
> Instead of the above anonymous struct, we can use the DECLARE_FLEX_ARRAY()
> helper here:
> 
> 		DECLARE_FLEX_ARRAY(struct cgroup, *ancestors);

Michal, can you update the patch with the above and resend?

Thanks.

-- 
tejun