cgroup: Replace cgroup->ancestor_ids[] with ->ancestors[]
authorTejun Heo <tj@kernel.org>
Fri, 29 Jul 2022 23:10:16 +0000 (13:10 -1000)
committerTejun Heo <tj@kernel.org>
Mon, 15 Aug 2022 21:16:47 +0000 (11:16 -1000)
Every cgroup knows all its ancestors through its ->ancestor_ids[]. There's
no advantage to remembering the IDs instead of the pointers directly and
this makes the array useless for finding an actual ancestor cgroup forcing
cgroup_ancestor() to iteratively walk up the hierarchy instead. Let's
replace cgroup->ancestor_ids[] with ->ancestors[] and remove the walking-up
from cgroup_ancestor().

While at it, improve comments around cgroup_root->cgrp_ancestor_storage.

This patch shouldn't cause user-visible behavior differences.

v2: Update cgroup_ancestor() to use ->ancestors[].

v3: cgroup_root->cgrp_ancestor_storage's type is updated to match
    cgroup->ancestors[]. Better comments.

Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
include/linux/cgroup-defs.h
include/linux/cgroup.h
kernel/cgroup/cgroup.c
net/netfilter/nft_socket.c
tools/perf/util/bpf_skel/bperf_cgroup.bpf.c

index 4bcf56b3491ca00bcc3276ddb2d63d0e28f4ee17..1283993d7ea8f30400a6e38cb1553e37cceae4b0 100644 (file)
@@ -384,7 +384,7 @@ struct cgroup {
        /*
         * The depth this cgroup is at.  The root is at depth zero and each
         * step down the hierarchy increments the level.  This along with
-        * ancestor_ids[] can determine whether a given cgroup is a
+        * ancestors[] can determine whether a given cgroup is a
         * descendant of another without traversing the hierarchy.
         */
        int level;
@@ -504,8 +504,8 @@ struct cgroup {
        /* Used to store internal freezer state */
        struct cgroup_freezer_state freezer;
 
-       /* ids of the ancestors at each level including self */
-       u64 ancestor_ids[];
+       /* All ancestors including self */
+       struct cgroup *ancestors[];
 };
 
 /*
@@ -522,11 +522,15 @@ struct cgroup_root {
        /* Unique id for this hierarchy. */
        int hierarchy_id;
 
-       /* The root cgroup.  Root is destroyed on its release. */
+       /*
+        * 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;
 
-       /* for cgrp->ancestor_ids[0] */
-       u64 cgrp_ancestor_id_storage;
+       /* 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;
index ed53bfe7c46c4738df18744cc45ff6a204721e11..4d143729b2468d0b29084b417efdfaab9f809718 100644 (file)
@@ -574,7 +574,7 @@ static inline bool cgroup_is_descendant(struct cgroup *cgrp,
 {
        if (cgrp->root != ancestor->root || cgrp->level < ancestor->level)
                return false;
-       return cgrp->ancestor_ids[ancestor->level] == cgroup_id(ancestor);
+       return cgrp->ancestors[ancestor->level] == ancestor;
 }
 
 /**
@@ -591,11 +591,9 @@ static inline bool cgroup_is_descendant(struct cgroup *cgrp,
 static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
                                             int ancestor_level)
 {
-       if (cgrp->level < ancestor_level)
+       if (ancestor_level < 0 || ancestor_level > cgrp->level)
                return NULL;
-       while (cgrp && cgrp->level > ancestor_level)
-               cgrp = cgroup_parent(cgrp);
-       return cgrp;
+       return cgrp->ancestors[ancestor_level];
 }
 
 /**
index ffaccd6373f1e786e9c90f8230b16f11ef4b8535..627ff0f07da753d0d7d45ea141f69b75b7a6ee45 100644 (file)
@@ -2049,7 +2049,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->ancestor_ids[0] = cgroup_id(root_cgrp);
+       root_cgrp->ancestors[0] = root_cgrp;
 
        ret = css_populate_dir(&root_cgrp->self);
        if (ret)
@@ -5400,8 +5400,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, ancestor_ids, (level + 1)),
-                      GFP_KERNEL);
+       cgrp = kzalloc(struct_size(cgrp, ancestors, (level + 1)), GFP_KERNEL);
        if (!cgrp)
                return ERR_PTR(-ENOMEM);
 
@@ -5453,7 +5452,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name,
 
        spin_lock_irq(&css_set_lock);
        for (tcgrp = cgrp; tcgrp; tcgrp = cgroup_parent(tcgrp)) {
-               cgrp->ancestor_ids[tcgrp->level] = cgroup_id(tcgrp);
+               cgrp->ancestors[tcgrp->level] = tcgrp;
 
                if (tcgrp != cgrp) {
                        tcgrp->nr_descendants++;
index a7de29137618ae2a2d7a2dccfb0a13c0c9503f8d..49a5348a6a14f08c7c67be724bbb5c1d9962e2c4 100644 (file)
@@ -40,16 +40,17 @@ static noinline bool
 nft_sock_get_eval_cgroupv2(u32 *dest, struct sock *sk, const struct nft_pktinfo *pkt, u32 level)
 {
        struct cgroup *cgrp;
+       u64 cgid;
 
        if (!sk_fullsock(sk))
                return false;
 
-       cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
-       if (level > cgrp->level)
+       cgrp = cgroup_ancestor(sock_cgroup_ptr(&sk->sk_cgrp_data), level);
+       if (!cgrp)
                return false;
 
-       memcpy(dest, &cgrp->ancestor_ids[level], sizeof(u64));
-
+       cgid = cgroup_id(cgrp);
+       memcpy(dest, &cgid, sizeof(u64));
        return true;
 }
 #endif
index 292c430768b525afdde468eca9b6ecea40955517..bd6a420acc8f110dc0bb284e76a1bc92733acfd0 100644 (file)
@@ -68,7 +68,7 @@ static inline int get_cgroup_v1_idx(__u32 *cgrps, int size)
                        break;
 
                // convert cgroup-id to a map index
-               cgrp_id = BPF_CORE_READ(cgrp, ancestor_ids[i]);
+               cgrp_id = BPF_CORE_READ(cgrp, ancestors[i], kn, id);
                elem = bpf_map_lookup_elem(&cgrp_idx, &cgrp_id);
                if (!elem)
                        continue;