[RFC PATCH bpf-next v2 0/9] bpf, cgroup: Add BPF support for cgroup1 hierarchy

[RFC PATCH bpf-next v2 0/9] bpf, cgroup: Add BPF support for cgroup1 hierarchy

[Yafang Shao]

Currently, BPF is primarily confined to cgroup2, with the exception of
cgroup_iter, which supports cgroup1 fds. Unfortunately, this limitation
prevents us from harnessing the full potential of BPF within cgroup1
environments.

In our endeavor to seamlessly integrate BPF within our Kubernetes
environment, which relies on cgroup1, we have been exploring the
possibility of transitioning to cgroup2. While this transition is
forward-looking, it poses challenges due to the necessity for numerous
applications to adapt.

While we acknowledge that cgroup2 represents the future, we also recognize
that such transitions demand time and effort. As a result, we are
considering an alternative approach. Instead of migrating to cgroup2, we
are contemplating modifications to the BPF kernel code to ensure
compatibility with cgroup1. These adjustments appear to be relatively
minor, making this option more feasible.

As discussed with Tejun[1], it has been determined that tying the interface
directly to the cgroup1 hierarchies is acceptable. As a result, this
patchset introduces cgroup1-only interfaces that operate with both
hierarchy ID and cgroup ID as parameters.

Within this patchset, a new cgroup1-only interface have been introduced,
which is also suggested by Tejun.

- [bpf_]task_get_cgroup1_within_hierarchy
  Acquires the associated cgroup of a task within a specific cgroup1
  hierarchy. The cgroup1 hierarchy is identified by its hierarchy ID.

This new kfunc enables the tracing of tasks within a designated container
or its ancestor cgroup directory in BPF programs. Additionally, it is
capable of operating on named cgroups, providing valuable utility for
hybrid cgroup mode scenarios.

To enable the use of this new kfunc in non-sleepable contexts, we need to
eliminate the reliance on the cgroup_mutex. Consequently, the cgroup
root_list is made RCU-safe, allowing us to replace the cgroup_mutex with
RCU read lock in specific paths. This enhancement can also bring
benefits to common operations in a production environment, such as
`cat /proc/self/cgroup`.

[1]. https://lwn.net/ml/cgroups/ZRHU6MfwqRxjBFUH@slm.duckdns.org/

Changes:
- RFC v1 -> RFC v2:
  - Introduce a new kunc to get cgroup kptr instead of getting the cgrp ID
    (Tejun)
  - Eliminate the cgroup_mutex by making cgroup root_list RCU-safe, as
    disccussed with Michal 
- RFC v1: bpf, cgroup: Add BPF support for cgroup1 hierarchy
  https://lwn.net/Articles/947130/
- bpf, cgroup: Add bpf support for cgroup controller
  https://lwn.net/Articles/945318/
- bpf, cgroup: Enable cgroup_array map on cgroup1
  https://lore.kernel.org/bpf/20230903142800.3870-1-laoar.shao@gmail.com

Yafang Shao (9):
  cgroup: Make operations on the cgroup root_list RCU safe
  cgroup: Eliminate the need for cgroup_mutex in proc_cgroup_show()
  cgroup: Add a new helper for cgroup1 hierarchy
  bpf: Add a new kfunc for cgroup1 hierarchy
  selftests/bpf: Fix issues in setup_classid_environment()
  selftests/bpf: Add parallel support for classid
  selftests/bpf: Add a new cgroup helper get_classid_cgroup_id()
  selftests/bpf: Add a new cgroup helper get_cgroup_hierarchy_id()
  selftests/bpf: Add selftests for cgroup1 hierarchy

 include/linux/cgroup-defs.h                   |   1 +
 include/linux/cgroup.h                        |   6 +-
 kernel/bpf/helpers.c                          |  20 +++
 kernel/cgroup/cgroup-internal.h               |   4 +-
 kernel/cgroup/cgroup-v1.c                     |  33 ++++
 kernel/cgroup/cgroup.c                        |  25 ++-
 tools/testing/selftests/bpf/cgroup_helpers.c  | 113 +++++++++++--
 tools/testing/selftests/bpf/cgroup_helpers.h  |   4 +-
 .../bpf/prog_tests/cgroup1_hierarchy.c        | 159 ++++++++++++++++++
 .../selftests/bpf/prog_tests/cgroup_v1v2.c    |   2 +-
 .../bpf/progs/test_cgroup1_hierarchy.c        |  73 ++++++++
 11 files changed, 409 insertions(+), 31 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup1_hierarchy.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c

-- 
2.30.1 (Apple Git-130)

[RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Yafang Shao]

At present, when we perform operations on the cgroup root_list, we must
hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
we can make operations on this list RCU-safe, eliminating the need to hold
the cgroup_mutex during traversal. Modifications to the list only occur in
the cgroup root setup and destroy paths, which should be infrequent in a
production environment. In contrast, traversal may occur frequently.
Therefore, making it RCU-safe would be beneficial.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/linux/cgroup-defs.h     |  1 +
 kernel/cgroup/cgroup-internal.h |  3 ++-
 kernel/cgroup/cgroup.c          | 17 ++++++++++-------
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index f1b3151ac30b..8505eeae6e41 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -558,6 +558,7 @@ struct cgroup_root {
 
 	/* A list running through the active hierarchies */
 	struct list_head root_list;
+	struct rcu_head rcu;
 
 	/* Hierarchy-specific flags */
 	unsigned int flags;
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index c56071f150f2..321af20ea15f 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -170,7 +170,8 @@ extern struct list_head cgroup_roots;
 
 /* iterate across the hierarchies */
 #define for_each_root(root)						\
-	list_for_each_entry((root), &cgroup_roots, root_list)
+	list_for_each_entry_rcu((root), &cgroup_roots, root_list,	\
+				!lockdep_is_held(&cgroup_mutex))
 
 /**
  * for_each_subsys - iterate all enabled cgroup subsystems
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 1fb7f562289d..bae8f9f27792 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1313,7 +1313,7 @@ static void cgroup_exit_root_id(struct cgroup_root *root)
 
 void cgroup_free_root(struct cgroup_root *root)
 {
-	kfree(root);
+	kfree_rcu(root, rcu);
 }
 
 static void cgroup_destroy_root(struct cgroup_root *root)
@@ -1346,7 +1346,7 @@ static void cgroup_destroy_root(struct cgroup_root *root)
 	spin_unlock_irq(&css_set_lock);
 
 	if (!list_empty(&root->root_list)) {
-		list_del(&root->root_list);
+		list_del_rcu(&root->root_list);
 		cgroup_root_count--;
 	}
 
@@ -1386,13 +1386,15 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
 		}
 	}
 
-	BUG_ON(!res_cgroup);
+	WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));
 	return res_cgroup;
 }
 
 /*
  * look up cgroup associated with current task's cgroup namespace on the
- * specified hierarchy
+ * specified hierarchy. Umount synchronization is ensured via VFS layer,
+ * so we don't have to hold cgroup_mutex to prevent the root from being
+ * destroyed.
  */
 static struct cgroup *
 current_cgns_cgroup_from_root(struct cgroup_root *root)
@@ -1445,7 +1447,6 @@ static struct cgroup *current_cgns_cgroup_dfl(void)
 static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
 					    struct cgroup_root *root)
 {
-	lockdep_assert_held(&cgroup_mutex);
 	lockdep_assert_held(&css_set_lock);
 
 	return __cset_cgroup_from_root(cset, root);
@@ -1453,7 +1454,9 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
 
 /*
  * Return the cgroup for "task" from the given hierarchy. Must be
- * called with cgroup_mutex and css_set_lock held.
+ * called with css_set_lock held to prevent task's groups from being modified.
+ * Must be called with either cgroup_mutex or rcu read lock to prevent the
+ * cgroup root from being destroyed.
  */
 struct cgroup *task_cgroup_from_root(struct task_struct *task,
 				     struct cgroup_root *root)
@@ -2097,7 +2100,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
 	 * care of subsystems' refcounts, which are explicitly dropped in
 	 * the failure exit path.
 	 */
-	list_add(&root->root_list, &cgroup_roots);
+	list_add_rcu(&root->root_list, &cgroup_roots);
 	cgroup_root_count++;
 
 	/*
-- 
2.30.1 (Apple Git-130)

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Michal Koutný]

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

On Tue, Oct 17, 2023 at 12:45:38PM +0000, Yafang Shao <laoar.shao@gmail.com> wrote:
> Therefore, making it RCU-safe would be beneficial.

Notice that whole cgroup_destroy_root() is actually an RCU callback (via
css_free_rwork_fn()). So the list traversal under RCU should alreay be
OK wrt its stability. Do you see a loophole in this argument?


>  /* iterate across the hierarchies */
>  #define for_each_root(root)						\
> -	list_for_each_entry((root), &cgroup_roots, root_list)
> +	list_for_each_entry_rcu((root), &cgroup_roots, root_list,	\
> +				!lockdep_is_held(&cgroup_mutex))

The extra condition should be constant false (regardless of
cgroup_mutex). IOW, RCU read lock is always required.

> @@ -1386,13 +1386,15 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
>  		}
>  	}
>  
> -	BUG_ON(!res_cgroup);
> +	WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));
>  	return res_cgroup;

Hm, this would benefit from a comment why !res_cgroup is conditionally
acceptable.

>  }
>  
>  /*
>   * look up cgroup associated with current task's cgroup namespace on the
> - * specified hierarchy
> + * specified hierarchy. Umount synchronization is ensured via VFS layer,
> + * so we don't have to hold cgroup_mutex to prevent the root from being
> + * destroyed.

I tried the similar via explicit lockdep invocation (in a thread I
linked to you previously) and VFS folks weren't ethusiastic about it.

You cannot hide this synchronization assumption in a mere comment.

Thanks,
Michal

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

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Yafang Shao]

On Tue, Oct 17, 2023 at 9:20 PM Michal Koutný <mkoutny@suse.com> wrote:
>
> On Tue, Oct 17, 2023 at 12:45:38PM +0000, Yafang Shao <laoar.shao@gmail.com> wrote:
> > Therefore, making it RCU-safe would be beneficial.
>
> Notice that whole cgroup_destroy_root() is actually an RCU callback (via
> css_free_rwork_fn()). So the list traversal under RCU should alreay be
> OK wrt its stability. Do you see a loophole in this argument?

css_free_rwork_fn() is designed for handling the cgroup's CSS. When
the RCU callback is executed, this specific CSS is not accessible to
other tasks. However, the cgroup root remains accessible to other
tasks. For instance, other tasks can still traverse the cgroup
root_list and access the cgroup root that is currently being
destroyed. Hence, we must take explicit measures to make access to the
cgroup root RCU-safe.

>
>
> >  /* iterate across the hierarchies */
> >  #define for_each_root(root)                                          \
> > -     list_for_each_entry((root), &cgroup_roots, root_list)
> > +     list_for_each_entry_rcu((root), &cgroup_roots, root_list,       \
> > +                             !lockdep_is_held(&cgroup_mutex))
>
> The extra condition should be constant false (regardless of
> cgroup_mutex). IOW, RCU read lock is always required.

IIUC, the RCU read lock is not required, while the cgroup_mutex is
required when we want to traverse the cgroup root_list, such as in the
case of cgroup_attach_task_all.

>
> > @@ -1386,13 +1386,15 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
> >               }
> >       }
> >
> > -     BUG_ON(!res_cgroup);
> > +     WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));
> >       return res_cgroup;
>
> Hm, this would benefit from a comment why !res_cgroup is conditionally
> acceptable.

The cgrp_cset_link is freed before we remove the cgroup root from the
root_list. Consequently, when accessing a cgroup root, the cset_link
may have already been freed, resulting in a NULL res_cgroup. However,
by holding the cgroup_mutex, we ensure that res_cgroup cannot be NULL.

Will add the comments in the next version.

>
> >  }
> >
> >  /*
> >   * look up cgroup associated with current task's cgroup namespace on the
> > - * specified hierarchy
> > + * specified hierarchy. Umount synchronization is ensured via VFS layer,
> > + * so we don't have to hold cgroup_mutex to prevent the root from being
> > + * destroyed.
>
> I tried the similar via explicit lockdep invocation (in a thread I
> linked to you previously) and VFS folks weren't ethusiastic about it.

Thanks for your information. will take a look at it.

>
> You cannot hide this synchronization assumption in a mere comment.

will think about how to address it.

--
Regards
Yafang

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Tejun Heo]

On Tue, Oct 17, 2023 at 12:45:38PM +0000, Yafang Shao wrote:
>  #define for_each_root(root)						\
> -	list_for_each_entry((root), &cgroup_roots, root_list)
> +	list_for_each_entry_rcu((root), &cgroup_roots, root_list,	\
> +				!lockdep_is_held(&cgroup_mutex))

Shouldn't that be lockdep_is_held() without the leading negation?

> @@ -1386,13 +1386,15 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
>  		}
>  	}
>  
> -	BUG_ON(!res_cgroup);
> +	WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));

This doesn't work. lockdep_is_held() is always true if !PROVE_LOCKING.

>  	return res_cgroup;
>  }
>  
>  /*
>   * look up cgroup associated with current task's cgroup namespace on the
> - * specified hierarchy
> + * specified hierarchy. Umount synchronization is ensured via VFS layer,
> + * so we don't have to hold cgroup_mutex to prevent the root from being
> + * destroyed.
>   */

Yeah, as Michal said, let's not do it this way.

Thanks.

-- 
tejun

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Yafang Shao]

On Wed, Oct 18, 2023 at 5:35 PM Tejun Heo <tj@kernel.org> wrote:
>
> On Tue, Oct 17, 2023 at 12:45:38PM +0000, Yafang Shao wrote:
> >  #define for_each_root(root)                                          \
> > -     list_for_each_entry((root), &cgroup_roots, root_list)
> > +     list_for_each_entry_rcu((root), &cgroup_roots, root_list,       \
> > +                             !lockdep_is_held(&cgroup_mutex))
>
> Shouldn't that be lockdep_is_held() without the leading negation?

right. will fix it.

>
> > @@ -1386,13 +1386,15 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
> >               }
> >       }
> >
> > -     BUG_ON(!res_cgroup);
> > +     WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));
>
> This doesn't work. lockdep_is_held() is always true if !PROVE_LOCKING.

will use mutex_is_locked() instead.

>
> >       return res_cgroup;
> >  }
> >
> >  /*
> >   * look up cgroup associated with current task's cgroup namespace on the
> > - * specified hierarchy
> > + * specified hierarchy. Umount synchronization is ensured via VFS layer,
> > + * so we don't have to hold cgroup_mutex to prevent the root from being
> > + * destroyed.
> >   */
>
> Yeah, as Michal said, let's not do it this way.

sure, will do it.

-- 
Regards
Yafang

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Tejun Heo]

On Thu, Oct 19, 2023 at 02:38:52PM +0800, Yafang Shao wrote:
> > > -     BUG_ON(!res_cgroup);
> > > +     WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));
> >
> > This doesn't work. lockdep_is_held() is always true if !PROVE_LOCKING.
> 
> will use mutex_is_locked() instead.

But then, someone else can hold the lock and trigger the condition
spuriously. The kernel doesn't track who's holding the lock unless lockdep
is enabled.

Thanks.

-- 
tejun

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Waiman Long]

On 10/19/23 15:08, Tejun Heo wrote:
> On Thu, Oct 19, 2023 at 02:38:52PM +0800, Yafang Shao wrote:
>>>> -     BUG_ON(!res_cgroup);
>>>> +     WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));
>>> This doesn't work. lockdep_is_held() is always true if !PROVE_LOCKING.
>> will use mutex_is_locked() instead.
> But then, someone else can hold the lock and trigger the condition
> spuriously. The kernel doesn't track who's holding the lock unless lockdep
> is enabled.

It is actually possible to detect if the current process is the owner of 
a mutex since there is a owner field in the mutex structure. However, 
the owner field also contains additional information which need to be 
masked off before comparing with "current". If such a functionality is 
really needed, we will have to add a helper function mutex_is_held(), 
for example, to kernel/locking/mutex.c.

Cheers,
Longman

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Yafang Shao]

On Fri, Oct 20, 2023 at 3:43 AM Waiman Long <longman@redhat.com> wrote:
>
> On 10/19/23 15:08, Tejun Heo wrote:
> > On Thu, Oct 19, 2023 at 02:38:52PM +0800, Yafang Shao wrote:
> >>>> -     BUG_ON(!res_cgroup);
> >>>> +     WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));
> >>> This doesn't work. lockdep_is_held() is always true if !PROVE_LOCKING.
> >> will use mutex_is_locked() instead.
> > But then, someone else can hold the lock and trigger the condition
> > spuriously. The kernel doesn't track who's holding the lock unless lockdep
> > is enabled.
>
> It is actually possible to detect if the current process is the owner of
> a mutex since there is a owner field in the mutex structure. However,
> the owner field also contains additional information which need to be
> masked off before comparing with "current". If such a functionality is
> really needed, we will have to add a helper function mutex_is_held(),
> for example, to kernel/locking/mutex.c.
、
Agreed. We should first introduce mutex_is_held(). Thanks for your suggestion.

-- 
Regards
Yafang

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Tejun Heo]

On Fri, Oct 20, 2023 at 05:36:57PM +0800, Yafang Shao wrote:
> On Fri, Oct 20, 2023 at 3:43 AM Waiman Long <longman@redhat.com> wrote:
> >
> > On 10/19/23 15:08, Tejun Heo wrote:
> > > On Thu, Oct 19, 2023 at 02:38:52PM +0800, Yafang Shao wrote:
> > >>>> -     BUG_ON(!res_cgroup);
> > >>>> +     WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));
> > >>> This doesn't work. lockdep_is_held() is always true if !PROVE_LOCKING.
> > >> will use mutex_is_locked() instead.
> > > But then, someone else can hold the lock and trigger the condition
> > > spuriously. The kernel doesn't track who's holding the lock unless lockdep
> > > is enabled.
> >
> > It is actually possible to detect if the current process is the owner of
> > a mutex since there is a owner field in the mutex structure. However,
> > the owner field also contains additional information which need to be
> > masked off before comparing with "current". If such a functionality is
> > really needed, we will have to add a helper function mutex_is_held(),
> > for example, to kernel/locking/mutex.c.
> 、
> Agreed. We should first introduce mutex_is_held(). Thanks for your suggestion.

I'm not sure this is the right occassion to add such thing. It's just a
warn_on, we can either pass in the necessary condition from the callers or
just drop the warning.

Thanks.

-- 
tejun

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[Yafang Shao]

On Sat, Oct 21, 2023 at 1:51 AM Tejun Heo <tj@kernel.org> wrote:
>
> On Fri, Oct 20, 2023 at 05:36:57PM +0800, Yafang Shao wrote:
> > On Fri, Oct 20, 2023 at 3:43 AM Waiman Long <longman@redhat.com> wrote:
> > >
> > > On 10/19/23 15:08, Tejun Heo wrote:
> > > > On Thu, Oct 19, 2023 at 02:38:52PM +0800, Yafang Shao wrote:
> > > >>>> -     BUG_ON(!res_cgroup);
> > > >>>> +     WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex));
> > > >>> This doesn't work. lockdep_is_held() is always true if !PROVE_LOCKING.
> > > >> will use mutex_is_locked() instead.
> > > > But then, someone else can hold the lock and trigger the condition
> > > > spuriously. The kernel doesn't track who's holding the lock unless lockdep
> > > > is enabled.
> > >
> > > It is actually possible to detect if the current process is the owner of
> > > a mutex since there is a owner field in the mutex structure. However,
> > > the owner field also contains additional information which need to be
> > > masked off before comparing with "current". If such a functionality is
> > > really needed, we will have to add a helper function mutex_is_held(),
> > > for example, to kernel/locking/mutex.c.
> > 、
> > Agreed. We should first introduce mutex_is_held(). Thanks for your suggestion.
>
> I'm not sure this is the right occassion to add such thing. It's just a
> warn_on, we can either pass in the necessary condition from the callers or
> just drop the warning.

OK. will just drop the warning.

-- 
Regards
Yafang

Re: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

[kernel test robot]

Hello,

kernel test robot noticed "WARNING:suspicious_RCU_usage" on:

commit: bf652e038501425f21e30c78c210ce892b9747c5 ("[RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe")
url: https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-Make-operations-on-the-cgroup-root_list-RCU-safe/20231017-204729
base: https://git.kernel.org/cgit/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/all/20231017124546.24608-2-laoar.shao@gmail.com/
patch subject: [RFC PATCH bpf-next v2 1/9] cgroup: Make operations on the cgroup root_list RCU safe

in testcase: boot

compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+------------------------------------------------------------------+------------+------------+
|                                                                  | 137df1189d | bf652e0385 |
+------------------------------------------------------------------+------------+------------+
| WARNING:suspicious_RCU_usage                                     | 0          | 6          |
| kernel/cgroup/cgroup#c:#RCU-list_traversed_in_non-reader_section | 0          | 6          |
+------------------------------------------------------------------+------------+------------+


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 <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202310251500.9683d034-oliver.sang@intel.com


[  245.792697][    T1] WARNING: suspicious RCU usage
[  245.793945][    T1] 6.6.0-rc5-01395-gbf652e038501 #1 Not tainted
[  245.795294][    T1] -----------------------------
[  245.796458][    T1] kernel/cgroup/cgroup-v1.c:1171 RCU-list traversed in non-reader section!!
[  245.798005][    T1]
[  245.798005][    T1] other info that might help us debug this:
[  245.798005][    T1]
[  245.800905][    T1]
[  245.800905][    T1] rcu_scheduler_active = 2, debug_locks = 1
[  245.815563][    T1] 1 lock held by init/1:
[ 245.816666][ T1] #0: c3bfffb8 (cgroup_mutex){+.+.}-{3:3}, at: cgroup_lock_and_drain_offline (kernel/cgroup/cgroup.c:3066) 
[  245.818306][    T1]
[  245.818306][    T1] stack backtrace:
[  245.820360][    T1] CPU: 0 PID: 1 Comm: init Not tainted 6.6.0-rc5-01395-gbf652e038501 #1
[  245.821860][    T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[  245.823506][    T1] Call Trace:
[ 245.824459][ T1] dump_stack_lvl (lib/dump_stack.c:107) 
[ 245.825654][ T1] dump_stack (lib/dump_stack.c:114) 
[ 245.826665][ T1] lockdep_rcu_suspicious (include/linux/context_tracking.h:153 kernel/locking/lockdep.c:6712) 
[ 245.827845][ T1] cgroup1_root_to_use (kernel/cgroup/cgroup-v1.c:1171 (discriminator 9)) 
[ 245.828999][ T1] cgroup1_get_tree (kernel/cgroup/cgroup-v1.c:1245) 
[ 245.830101][ T1] vfs_get_tree (fs/super.c:1750) 
[ 245.831172][ T1] do_new_mount (fs/namespace.c:3335) 
[ 245.832199][ T1] path_mount (fs/namespace.c:3662) 
[ 245.833147][ T1] ? user_path_at_empty (fs/namei.c:2914) 
[ 245.834198][ T1] __ia32_sys_mount (fs/namespace.c:3675 fs/namespace.c:3884 fs/namespace.c:3861 fs/namespace.c:3861) 
[ 245.835242][ T1] do_int80_syscall_32 (arch/x86/entry/common.c:112 arch/x86/entry/common.c:132) 
[ 245.836343][ T1] ? kfree (mm/slab_common.c:1073) 
[ 245.837335][ T1] ? __ia32_sys_mount (fs/namespace.c:3861) 
[ 245.838404][ T1] ? do_int80_syscall_32 (arch/x86/entry/common.c:136) 
[ 245.839479][ T1] ? syscall_exit_to_user_mode (kernel/entry/common.c:131 kernel/entry/common.c:298) 
[ 245.840593][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:4565) 
[ 245.841824][ T1] ? syscall_exit_to_user_mode (kernel/entry/common.c:299) 
[ 245.842967][ T1] ? do_int80_syscall_32 (arch/x86/entry/common.c:136) 
[ 245.843964][ T1] ? __ia32_sys_mount (fs/namespace.c:3861) 
[ 245.844935][ T1] ? do_int80_syscall_32 (arch/x86/entry/common.c:136) 
[ 245.845958][ T1] ? syscall_exit_to_user_mode (kernel/entry/common.c:131 kernel/entry/common.c:298) 
[ 245.847004][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:4565) 
[ 245.848103][ T1] ? syscall_exit_to_user_mode (kernel/entry/common.c:299) 
[ 245.849159][ T1] ? do_int80_syscall_32 (arch/x86/entry/common.c:136) 
[ 245.850163][ T1] ? syscall_exit_to_user_mode (kernel/entry/common.c:299) 
[ 245.851209][ T1] ? do_int80_syscall_32 (arch/x86/entry/common.c:136) 
[ 245.852179][ T1] ? lock_release (kernel/locking/lockdep.c:467 kernel/locking/lockdep.c:5775) 
[ 245.853126][ T1] ? irqentry_exit (kernel/entry/common.c:445) 
[ 245.858431][ T1] ? irqentry_exit_to_user_mode (kernel/entry/common.c:131 kernel/entry/common.c:311) 
[ 245.859731][ T1] ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:4565) 
[ 245.860985][ T1] ? irqentry_exit_to_user_mode (kernel/entry/common.c:312) 
[ 245.862110][ T1] ? irqentry_exit (kernel/entry/common.c:445) 
[ 245.863099][ T1] ? exc_page_fault (arch/x86/mm/fault.c:1565) 
[ 245.864073][ T1] entry_INT80_32 (arch/x86/entry/entry_32.S:947) 
[  245.865070][    T1] EIP: 0xb7f895ed
[ 245.865966][ T1] Code: 8b 7c 24 0c 50 e8 06 00 00 00 89 da 5b 5b 5f c3 8b 04 24 05 77 ec 04 00 8b 00 85 c0 74 06 50 8b 44 24 08 c3 8b 44 24 04 cd 80 <c3> 55 50 8b 6c 24 0c 8b 45 00 8b 6d 04 50 8b 44 24 04 e8 b9 ff ff
All code
========
   0:	8b 7c 24 0c          	mov    0xc(%rsp),%edi
   4:	50                   	push   %rax
   5:	e8 06 00 00 00       	call   0x10
   a:	89 da                	mov    %ebx,%edx
   c:	5b                   	pop    %rbx
   d:	5b                   	pop    %rbx
   e:	5f                   	pop    %rdi
   f:	c3                   	ret
  10:	8b 04 24             	mov    (%rsp),%eax
  13:	05 77 ec 04 00       	add    $0x4ec77,%eax
  18:	8b 00                	mov    (%rax),%eax
  1a:	85 c0                	test   %eax,%eax
  1c:	74 06                	je     0x24
  1e:	50                   	push   %rax
  1f:	8b 44 24 08          	mov    0x8(%rsp),%eax
  23:	c3                   	ret
  24:	8b 44 24 04          	mov    0x4(%rsp),%eax
  28:	cd 80                	int    $0x80
  2a:*	c3                   	ret		<-- trapping instruction
  2b:	55                   	push   %rbp
  2c:	50                   	push   %rax
  2d:	8b 6c 24 0c          	mov    0xc(%rsp),%ebp
  31:	8b 45 00             	mov    0x0(%rbp),%eax
  34:	8b 6d 04             	mov    0x4(%rbp),%ebp
  37:	50                   	push   %rax
  38:	8b 44 24 04          	mov    0x4(%rsp),%eax
  3c:	e8                   	.byte 0xe8
  3d:	b9                   	.byte 0xb9
  3e:	ff                   	(bad)
  3f:	ff                   	.byte 0xff

Code starting with the faulting instruction
===========================================
   0:	c3                   	ret
   1:	55                   	push   %rbp
   2:	50                   	push   %rax
   3:	8b 6c 24 0c          	mov    0xc(%rsp),%ebp
   7:	8b 45 00             	mov    0x0(%rbp),%eax
   a:	8b 6d 04             	mov    0x4(%rbp),%ebp
   d:	50                   	push   %rax
   e:	8b 44 24 04          	mov    0x4(%rsp),%eax
  12:	e8                   	.byte 0xe8
  13:	b9                   	.byte 0xb9
  14:	ff                   	(bad)
  15:	ff                   	.byte 0xff
[  245.868936][    T1] EAX: ffffffda EBX: 0804a431 ECX: 0804a429 EDX: 0804a431
[  245.870279][    T1] ESI: 0000000e EDI: 00000000 EBP: bffa7bd8 ESP: bffa7bb8
[  245.871623][    T1] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000246
[  245.935209][    T1] init: Console is alive



The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20231025/202310251500.9683d034-oliver.sang@intel.com



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

[RFC PATCH bpf-next v2 2/9] cgroup: Eliminate the need for cgroup_mutex in proc_cgroup_show()

[Yafang Shao]

The cgroup root_list is already RCU-safe. Therefore, we can replace the
cgroup_mutex with the RCU read lock in some particular paths. This change
will be particularly beneficial for frequent operations, such as
`cat /proc/self/cgroup`, in a cgroup1-based container environment.

I did stress tests with this change, as outlined below
(with CONFIG_PROVE_RCU_LIST enabled):

- Continuously mounting and unmounting named cgroups in some tasks,
  for example:

  cgrp_name=$1
  while true
  do
      mount -t cgroup -o none,name=$cgrp_name none /$cgrp_name
      umount /$cgrp_name
  done

- Continuously triggering proc_cgroup_show() in some tasks concurrently,
  for example:
  while true; do cat /proc/self/cgroup > /dev/null; done

They can ran successfully after implementing this change, with no RCU
warnings in dmesg. It's worth noting that this change can also catch
deleted cgroups, as demonstrated by running the following task at the
same time:

  while true; do grep deleted /proc/self/cgroup; done

Results in output like:

  7995:name=cgrp2: (deleted)
  8594:name=cgrp1: (deleted)

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/cgroup/cgroup.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index bae8f9f27792..30bdb3bf1dcd 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6256,7 +6256,7 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 	if (!buf)
 		goto out;
 
-	cgroup_lock();
+	rcu_read_lock();
 	spin_lock_irq(&css_set_lock);
 
 	for_each_root(root) {
@@ -6279,6 +6279,10 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 		seq_putc(m, ':');
 
 		cgrp = task_cgroup_from_root(tsk, root);
+		if (!cgrp) {
+			seq_puts(m, " (deleted)\n");
+			continue;
+		}
 
 		/*
 		 * On traditional hierarchies, all zombie tasks show up as
@@ -6311,7 +6315,7 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 	retval = 0;
 out_unlock:
 	spin_unlock_irq(&css_set_lock);
-	cgroup_unlock();
+	rcu_read_unlock();
 	kfree(buf);
 out:
 	return retval;
-- 
2.30.1 (Apple Git-130)

Re: [RFC PATCH bpf-next v2 2/9] cgroup: Eliminate the need for cgroup_mutex in proc_cgroup_show()

[Michal Koutný]

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

Hi.

I'd like this proc_cgroup_show de-contention.
(Provided the previous patch and this one can be worked out somehow.)

On Tue, Oct 17, 2023 at 12:45:39PM +0000, Yafang Shao <laoar.shao@gmail.com> wrote:
> They can ran successfully after implementing this change, with no RCU
> warnings in dmesg. It's worth noting that this change can also catch
> deleted cgroups, as demonstrated by running the following task at the
> same time:

Can those be other than v1 root cgroups? A suffix "(unmounted)" may be
more informative then.

(Non-zombie tasks prevent their cgroup removal, zombie tasks won't have
any non-trivial path rendered.)


> @@ -6256,7 +6256,7 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
>  	if (!buf)
>  		goto out;
>  
> -	cgroup_lock();
> +	rcu_read_lock();

What about the cgroup_path_ns_locked() that prints the path?

(I argue above that no non-trivial path is rendered but I'm not sure
whether rcu_read_lock() is sufficient sync wrt cgroup rmdir.)


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

Re: [RFC PATCH bpf-next v2 2/9] cgroup: Eliminate the need for cgroup_mutex in proc_cgroup_show()

[Yafang Shao]

On Tue, Oct 17, 2023 at 10:04 PM Michal Koutný <mkoutny@suse.com> wrote:
>
> Hi.
>
> I'd like this proc_cgroup_show de-contention.
> (Provided the previous patch and this one can be worked out somehow.)

Thanks

>
> On Tue, Oct 17, 2023 at 12:45:39PM +0000, Yafang Shao <laoar.shao@gmail.com> wrote:
> > They can ran successfully after implementing this change, with no RCU
> > warnings in dmesg. It's worth noting that this change can also catch
> > deleted cgroups, as demonstrated by running the following task at the
> > same time:
>
> Can those be other than v1 root cgroups? A suffix "(unmounted)" may be
> more informative then.

They can only be a v1 root cgroups. will use the "(unmounted)"
instead. Thanks for your suggestion.

>
> (Non-zombie tasks prevent their cgroup removal, zombie tasks won't have
> any non-trivial path rendered.)
>
>
> > @@ -6256,7 +6256,7 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
> >       if (!buf)
> >               goto out;
> >
> > -     cgroup_lock();
> > +     rcu_read_lock();
>
> What about the cgroup_path_ns_locked() that prints the path?

I believe we can further enhance cgroup_path_ns() by replacing the
cgroup_lock with rcu_read_lock. However, we need to explicitly address
the NULL root case, which may necessitate some refactoring. Perhaps
this can be improved in a separate patchset?

>
> (I argue above that no non-trivial path is rendered but I'm not sure
> whether rcu_read_lock() is sufficient sync wrt cgroup rmdir.)
>


-- 
Regards
Yafang

Re: [RFC PATCH bpf-next v2 2/9] cgroup: Eliminate the need for cgroup_mutex in proc_cgroup_show()

[Tejun Heo]

On Tue, Oct 17, 2023 at 12:45:39PM +0000, Yafang Shao wrote:
> Results in output like:
> 
>   7995:name=cgrp2: (deleted)
>   8594:name=cgrp1: (deleted)

Just skip them? What would userspace do with the above information?

Thanks.

-- 
tejun

Re: [RFC PATCH bpf-next v2 2/9] cgroup: Eliminate the need for cgroup_mutex in proc_cgroup_show()

[Yafang Shao]

On Wed, Oct 18, 2023 at 5:38 PM Tejun Heo <tj@kernel.org> wrote:
>
> On Tue, Oct 17, 2023 at 12:45:39PM +0000, Yafang Shao wrote:
> > Results in output like:
> >
> >   7995:name=cgrp2: (deleted)
> >   8594:name=cgrp1: (deleted)
>
> Just skip them? What would userspace do with the above information?

It should be useless to userspace.
will skip it.

-- 
Regards
Yafang

[RFC PATCH bpf-next v2 3/9] cgroup: Add a new helper for cgroup1 hierarchy

[Yafang Shao]

A new helper is added for cgroup1 hierarchy:

- task_get_cgroup1_within_hierarchy
  Acquires the associated cgroup of a task within a specific cgroup1
  hierarchy. The cgroup1 hierarchy is identified by its hierarchy ID.

This helper function is added to facilitate the tracing of tasks within
a particular container or cgroup dir in BPF programs. It's important to
note that this helper is designed specifically for cgroup1 only.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/linux/cgroup.h          |  6 +++++-
 kernel/cgroup/cgroup-internal.h |  1 -
 kernel/cgroup/cgroup-v1.c       | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index b307013b9c6c..0cd3983822c3 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -71,6 +71,7 @@ struct css_task_iter {
 extern struct file_system_type cgroup_fs_type;
 extern struct cgroup_root cgrp_dfl_root;
 extern struct css_set init_css_set;
+extern spinlock_t css_set_lock;
 
 #define SUBSYS(_x) extern struct cgroup_subsys _x ## _cgrp_subsys;
 #include <linux/cgroup_subsys.h>
@@ -159,6 +160,8 @@ void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags,
 			 struct css_task_iter *it);
 struct task_struct *css_task_iter_next(struct css_task_iter *it);
 void css_task_iter_end(struct css_task_iter *it);
+struct cgroup *task_cgroup_from_root(struct task_struct *task,
+				     struct cgroup_root *root);
 
 /**
  * css_for_each_child - iterate through children of a css
@@ -388,7 +391,6 @@ static inline void cgroup_unlock(void)
  * as locks used during the cgroup_subsys::attach() methods.
  */
 #ifdef CONFIG_PROVE_RCU
-extern spinlock_t css_set_lock;
 #define task_css_set_check(task, __c)					\
 	rcu_dereference_check((task)->cgroups,				\
 		rcu_read_lock_sched_held() ||				\
@@ -855,4 +857,6 @@ static inline void cgroup_bpf_put(struct cgroup *cgrp) {}
 
 #endif /* CONFIG_CGROUP_BPF */
 
+struct cgroup *task_get_cgroup1_within_hierarchy(struct task_struct *tsk, int hierarchy_id);
+
 #endif /* _LINUX_CGROUP_H */
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 321af20ea15f..3a7b0cc911f7 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -164,7 +164,6 @@ struct cgroup_mgctx {
 #define DEFINE_CGROUP_MGCTX(name)						\
 	struct cgroup_mgctx name = CGROUP_MGCTX_INIT(name)
 
-extern spinlock_t css_set_lock;
 extern struct cgroup_subsys *cgroup_subsys[];
 extern struct list_head cgroup_roots;
 
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index c487ffef6652..571071d45e4d 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -1263,6 +1263,39 @@ int cgroup1_get_tree(struct fs_context *fc)
 	return ret;
 }
 
+/**
+ * task_cgroup_id_within_hierarchy - Acquires the associated cgroup of a task
+ * within a specific cgroup1 hierarchy. The cgroup1 hierarchy is identified by
+ * its hierarchy ID.
+ * @tsk: The target task
+ * @hierarchy_id: The ID of a cgroup1 hierarchy
+ *
+ * On success, the cgroup is returned. On failure, ERR_PTR is returned.
+ * We limit it to cgroup1 only.
+ */
+struct cgroup *task_get_cgroup1_within_hierarchy(struct task_struct *tsk, int hierarchy_id)
+{
+	struct cgroup *cgrp = ERR_PTR(-ENOENT);
+	struct cgroup_root *root;
+
+	rcu_read_lock();
+	list_for_each_entry(root, &cgroup_roots, root_list) {
+		/* cgroup1 only*/
+		if (root == &cgrp_dfl_root)
+			continue;
+		if (root->hierarchy_id != hierarchy_id)
+			continue;
+		spin_lock_irq(&css_set_lock);
+		cgrp = task_cgroup_from_root(tsk, root);
+		if (!cgrp || !cgroup_tryget(cgrp))
+			cgrp = ERR_PTR(-ENOENT);
+		spin_unlock_irq(&css_set_lock);
+		break;
+	}
+	rcu_read_unlock();
+	return cgrp;
+}
+
 static int __init cgroup1_wq_init(void)
 {
 	/*
-- 
2.30.1 (Apple Git-130)

Re: [RFC PATCH bpf-next v2 3/9] cgroup: Add a new helper for cgroup1 hierarchy

[Tejun Heo]

On Tue, Oct 17, 2023 at 12:45:40PM +0000, Yafang Shao wrote:
> +/**
> + * task_cgroup_id_within_hierarchy - Acquires the associated cgroup of a task
      ^
      doesn't match actual function name.

> + * within a specific cgroup1 hierarchy. The cgroup1 hierarchy is identified by
> + * its hierarchy ID.
> + * @tsk: The target task
> + * @hierarchy_id: The ID of a cgroup1 hierarchy
> + *
> + * On success, the cgroup is returned. On failure, ERR_PTR is returned.
> + * We limit it to cgroup1 only.
> + */
> +struct cgroup *task_get_cgroup1_within_hierarchy(struct task_struct *tsk, int hierarchy_id)

Maybe we can just named it task_get_cgroup1()?

Thanks.

-- 
tejun

Re: [RFC PATCH bpf-next v2 3/9] cgroup: Add a new helper for cgroup1 hierarchy

[Yafang Shao]

On Wed, Oct 18, 2023 at 5:44 PM Tejun Heo <tj@kernel.org> wrote:
>
> On Tue, Oct 17, 2023 at 12:45:40PM +0000, Yafang Shao wrote:
> > +/**
> > + * task_cgroup_id_within_hierarchy - Acquires the associated cgroup of a task
>       ^
>       doesn't match actual function name.

Ah. forgot to change it.
will fix it.

>
> > + * within a specific cgroup1 hierarchy. The cgroup1 hierarchy is identified by
> > + * its hierarchy ID.
> > + * @tsk: The target task
> > + * @hierarchy_id: The ID of a cgroup1 hierarchy
> > + *
> > + * On success, the cgroup is returned. On failure, ERR_PTR is returned.
> > + * We limit it to cgroup1 only.
> > + */
> > +struct cgroup *task_get_cgroup1_within_hierarchy(struct task_struct *tsk, int hierarchy_id)
>
> Maybe we can just named it task_get_cgroup1()?

sure. will use it instead.

-- 
Regards
Yafang

[RFC PATCH bpf-next v2 4/9] bpf: Add a new kfunc for cgroup1 hierarchy

[Yafang Shao]

A new kfunc is added to acquire cgroup1:

- bpf_task_get_cgroup1_within_hierarchy
  Acquires the associated cgroup of a task whithin a specific cgroup1
  hierarchy. The cgroup1 hierarchy is identified by its hierarchy ID.

This new kfunc enables the tracing of tasks within a designated
container or cgroup directory in BPF programs.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/bpf/helpers.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 62a53ebfedf9..a682b47d3b97 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2217,6 +2217,25 @@ __bpf_kfunc long bpf_task_under_cgroup(struct task_struct *task,
 {
 	return task_under_cgroup_hierarchy(task, ancestor);
 }
+
+/**
+ * bpf_task_get_cgroup_within_hierarchy - Acquires the associated cgroup of
+ * a task within a specific cgroup1 hierarchy. The cgroup1 hierarchy is
+ * identified by its hierarchy ID.
+ * @task: The target task
+ * @hierarchy_id: The ID of a cgroup1 hierarchy
+ *
+ * On success, the cgroup is returen. On failure, NULL is returned.
+ */
+__bpf_kfunc struct cgroup *
+bpf_task_get_cgroup1_within_hierarchy(struct task_struct *task, int hierarchy_id)
+{
+	struct cgroup *cgrp = task_get_cgroup1_within_hierarchy(task, hierarchy_id);
+
+	if (IS_ERR(cgrp))
+		return NULL;
+	return cgrp;
+}
 #endif /* CONFIG_CGROUPS */
 
 /**
@@ -2523,6 +2542,7 @@ BTF_ID_FLAGS(func, bpf_cgroup_release, KF_RELEASE)
 BTF_ID_FLAGS(func, bpf_cgroup_ancestor, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_cgroup_from_id, KF_ACQUIRE | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_task_under_cgroup, KF_RCU)
+BTF_ID_FLAGS(func, bpf_task_get_cgroup1_within_hierarchy, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
 #endif
 BTF_ID_FLAGS(func, bpf_task_from_pid, KF_ACQUIRE | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_throw)
-- 
2.30.1 (Apple Git-130)

Re: [RFC PATCH bpf-next v2 4/9] bpf: Add a new kfunc for cgroup1 hierarchy

[Tejun Heo]

One nit.

On Tue, Oct 17, 2023 at 12:45:41PM +0000, Yafang Shao wrote:
> +/**
> + * bpf_task_get_cgroup_within_hierarchy - Acquires the associated cgroup of
                         ^
			 1

Thanks.

-- 
tejun

Re: [RFC PATCH bpf-next v2 4/9] bpf: Add a new kfunc for cgroup1 hierarchy

[Yafang Shao]

On Wed, Oct 18, 2023 at 5:40 PM Tejun Heo <tj@kernel.org> wrote:
>
> One nit.
>
> On Tue, Oct 17, 2023 at 12:45:41PM +0000, Yafang Shao wrote:
> > +/**
> > + * bpf_task_get_cgroup_within_hierarchy - Acquires the associated cgroup of
>                          ^
>                          1

Thanks for pointing it out.
will fix it.

-- 
Regards
Yafang

[RFC PATCH bpf-next v2 5/9] selftests/bpf: Fix issues in setup_classid_environment()

[Yafang Shao]

If the net_cls subsystem is already mounted, attempting to mount it again
in setup_classid_environment() will result in a failure with the error code
EBUSY. Despite this, tmpfs will have been successfully mounted at
/sys/fs/cgroup/net_cls. Consequently, the /sys/fs/cgroup/net_cls directory
will be empty, causing subsequent setup operations to fail.

Here's an error log excerpt illustrating the issue when net_cls has already
been mounted at /sys/fs/cgroup/net_cls prior to running
setup_classid_environment():

- Before that change

  $ tools/testing/selftests/bpf/test_progs --name=cgroup_v1v2
  test_cgroup_v1v2:PASS:server_fd 0 nsec
  test_cgroup_v1v2:PASS:client_fd 0 nsec
  test_cgroup_v1v2:PASS:cgroup_fd 0 nsec
  test_cgroup_v1v2:PASS:server_fd 0 nsec
  run_test:PASS:skel_open 0 nsec
  run_test:PASS:prog_attach 0 nsec
  test_cgroup_v1v2:PASS:cgroup-v2-only 0 nsec
  (cgroup_helpers.c:248: errno: No such file or directory) Opening Cgroup Procs: /sys/fs/cgroup/net_cls/cgroup.procs
  (cgroup_helpers.c:540: errno: No such file or directory) Opening cgroup classid: /sys/fs/cgroup/net_cls/cgroup-test-work-dir/net_cls.classid
  run_test:PASS:skel_open 0 nsec
  run_test:PASS:prog_attach 0 nsec
  (cgroup_helpers.c:248: errno: No such file or directory) Opening Cgroup Procs: /sys/fs/cgroup/net_cls/cgroup-test-work-dir/cgroup.procs
  run_test:FAIL:join_classid unexpected error: 1 (errno 2)
  test_cgroup_v1v2:FAIL:cgroup-v1v2 unexpected error: -1 (errno 2)
  (cgroup_helpers.c:248: errno: No such file or directory) Opening Cgroup Procs: /sys/fs/cgroup/net_cls/cgroup.procs
  #44      cgroup_v1v2:FAIL
  Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED

- After that change
  $ tools/testing/selftests/bpf/test_progs --name=cgroup_v1v2
  #44      cgroup_v1v2:OK
  Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 tools/testing/selftests/bpf/cgroup_helpers.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/cgroup_helpers.c b/tools/testing/selftests/bpf/cgroup_helpers.c
index 5b1da2a32ea7..10b5f42e65e7 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.c
+++ b/tools/testing/selftests/bpf/cgroup_helpers.c
@@ -523,10 +523,20 @@ int setup_classid_environment(void)
 		return 1;
 	}
 
-	if (mount("net_cls", NETCLS_MOUNT_PATH, "cgroup", 0, "net_cls") &&
-	    errno != EBUSY) {
-		log_err("mount cgroup net_cls");
-		return 1;
+	if (mount("net_cls", NETCLS_MOUNT_PATH, "cgroup", 0, "net_cls")) {
+		if (errno != EBUSY) {
+			log_err("mount cgroup net_cls");
+			return 1;
+		}
+
+		if (rmdir(NETCLS_MOUNT_PATH)) {
+			log_err("rmdir cgroup net_cls");
+			return 1;
+		}
+		if (umount(CGROUP_MOUNT_DFLT)) {
+			log_err("umount cgroup base");
+			return 1;
+		}
 	}
 
 	cleanup_classid_environment();
-- 
2.30.1 (Apple Git-130)

[RFC PATCH bpf-next v2 6/9] selftests/bpf: Add parallel support for classid

[Yafang Shao]

Include the current pid in the classid cgroup path. This way, different
testers relying on classid-based configurations will have distinct classid
cgroup directories, enabling them to run concurrently. Additionally, we
leverage the current pid as the classid, ensuring unique identification.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 tools/testing/selftests/bpf/cgroup_helpers.c   | 18 +++++++++++-------
 tools/testing/selftests/bpf/cgroup_helpers.h   |  2 +-
 .../selftests/bpf/prog_tests/cgroup_v1v2.c     |  2 +-
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/cgroup_helpers.c b/tools/testing/selftests/bpf/cgroup_helpers.c
index 10b5f42e65e7..f18649a79d64 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.c
+++ b/tools/testing/selftests/bpf/cgroup_helpers.c
@@ -45,9 +45,12 @@
 #define format_parent_cgroup_path(buf, path) \
 	format_cgroup_path_pid(buf, path, getppid())
 
-#define format_classid_path(buf)				\
-	snprintf(buf, sizeof(buf), "%s%s", NETCLS_MOUNT_PATH,	\
-		 CGROUP_WORK_DIR)
+#define format_classid_path_pid(buf, pid)				\
+	snprintf(buf, sizeof(buf), "%s%s%d", NETCLS_MOUNT_PATH,	\
+		 CGROUP_WORK_DIR, pid)
+
+#define format_classid_path(buf)	\
+	format_classid_path_pid(buf, getpid())
 
 static __thread bool cgroup_workdir_mounted;
 
@@ -551,15 +554,16 @@ int setup_classid_environment(void)
 
 /**
  * set_classid() - Set a cgroupv1 net_cls classid
- * @id: the numeric classid
  *
- * Writes the passed classid into the cgroup work dir's net_cls.classid
+ * Writes the classid into the cgroup work dir's net_cls.classid
  * file in order to later on trigger socket tagging.
  *
+ * We leverage the current pid as the classid, ensuring unique identification.
+ *
  * On success, it returns 0, otherwise on failure it returns 1. If there
  * is a failure, it prints the error to stderr.
  */
-int set_classid(unsigned int id)
+int set_classid(void)
 {
 	char cgroup_workdir[PATH_MAX - 42];
 	char cgroup_classid_path[PATH_MAX + 1];
@@ -575,7 +579,7 @@ int set_classid(unsigned int id)
 		return 1;
 	}
 
-	if (dprintf(fd, "%u\n", id) < 0) {
+	if (dprintf(fd, "%u\n", getpid()) < 0) {
 		log_err("Setting cgroup classid");
 		rc = 1;
 	}
diff --git a/tools/testing/selftests/bpf/cgroup_helpers.h b/tools/testing/selftests/bpf/cgroup_helpers.h
index 5c2cb9c8b546..92fc41daf4a4 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.h
+++ b/tools/testing/selftests/bpf/cgroup_helpers.h
@@ -29,7 +29,7 @@ int setup_cgroup_environment(void);
 void cleanup_cgroup_environment(void);
 
 /* cgroupv1 related */
-int set_classid(unsigned int id);
+int set_classid(void);
 int join_classid(void);
 
 int setup_classid_environment(void);
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c
index 9026b42914d3..addf720428f7 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c
@@ -71,7 +71,7 @@ void test_cgroup_v1v2(void)
 	}
 	ASSERT_OK(run_test(cgroup_fd, server_fd, false), "cgroup-v2-only");
 	setup_classid_environment();
-	set_classid(42);
+	set_classid();
 	ASSERT_OK(run_test(cgroup_fd, server_fd, true), "cgroup-v1v2");
 	cleanup_classid_environment();
 	close(server_fd);
-- 
2.30.1 (Apple Git-130)

[RFC PATCH bpf-next v2 7/9] selftests/bpf: Add a new cgroup helper get_classid_cgroup_id()

[Yafang Shao]

Introduce a new helper function to retrieve the cgroup ID from a net_cls
cgroup directory.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 tools/testing/selftests/bpf/cgroup_helpers.c | 28 +++++++++++++++-----
 tools/testing/selftests/bpf/cgroup_helpers.h |  1 +
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/cgroup_helpers.c b/tools/testing/selftests/bpf/cgroup_helpers.c
index f18649a79d64..63bfa72185be 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.c
+++ b/tools/testing/selftests/bpf/cgroup_helpers.c
@@ -422,26 +422,23 @@ int create_and_get_cgroup(const char *relative_path)
 }
 
 /**
- * get_cgroup_id() - Get cgroup id for a particular cgroup path
- * @relative_path: The cgroup path, relative to the workdir, to join
+ * get_cgroup_id_from_path - Get cgroup id for a particular cgroup path
+ * @cgroup_workdir: The absolute cgroup path
  *
  * On success, it returns the cgroup id. On failure it returns 0,
  * which is an invalid cgroup id.
  * If there is a failure, it prints the error to stderr.
  */
-unsigned long long get_cgroup_id(const char *relative_path)
+unsigned long long get_cgroup_id_from_path(const char *cgroup_workdir)
 {
 	int dirfd, err, flags, mount_id, fhsize;
 	union {
 		unsigned long long cgid;
 		unsigned char raw_bytes[8];
 	} id;
-	char cgroup_workdir[PATH_MAX + 1];
 	struct file_handle *fhp, *fhp2;
 	unsigned long long ret = 0;
 
-	format_cgroup_path(cgroup_workdir, relative_path);
-
 	dirfd = AT_FDCWD;
 	flags = 0;
 	fhsize = sizeof(*fhp);
@@ -477,6 +474,14 @@ unsigned long long get_cgroup_id(const char *relative_path)
 	return ret;
 }
 
+unsigned long long get_cgroup_id(const char *relative_path)
+{
+	char cgroup_workdir[PATH_MAX + 1];
+
+	format_cgroup_path(cgroup_workdir, relative_path);
+	return get_cgroup_id_from_path(cgroup_workdir);
+}
+
 int cgroup_setup_and_join(const char *path) {
 	int cg_fd;
 
@@ -621,3 +626,14 @@ void cleanup_classid_environment(void)
 	join_cgroup_from_top(NETCLS_MOUNT_PATH);
 	nftw(cgroup_workdir, nftwfunc, WALK_FD_LIMIT, FTW_DEPTH | FTW_MOUNT);
 }
+
+/**
+ * get_classid_cgroup_id - Get the cgroup id of a net_cls cgroup
+ */
+unsigned long long get_classid_cgroup_id(void)
+{
+	char cgroup_workdir[PATH_MAX + 1];
+
+	format_classid_path(cgroup_workdir);
+	return get_cgroup_id_from_path(cgroup_workdir);
+}
diff --git a/tools/testing/selftests/bpf/cgroup_helpers.h b/tools/testing/selftests/bpf/cgroup_helpers.h
index 92fc41daf4a4..e71da4ef031b 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.h
+++ b/tools/testing/selftests/bpf/cgroup_helpers.h
@@ -31,6 +31,7 @@ void cleanup_cgroup_environment(void);
 /* cgroupv1 related */
 int set_classid(void);
 int join_classid(void);
+unsigned long long get_classid_cgroup_id(void);
 
 int setup_classid_environment(void);
 void cleanup_classid_environment(void);
-- 
2.30.1 (Apple Git-130)

[RFC PATCH bpf-next v2 8/9] selftests/bpf: Add a new cgroup helper get_cgroup_hierarchy_id()

[Yafang Shao]

A new cgroup helper function, get_cgroup1_hierarchy_id(), has been
introduced to obtain the ID of a cgroup1 hierarchy based on the provided
cgroup name. This cgroup name can be obtained from the /proc/self/cgroup
file.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 tools/testing/selftests/bpf/cgroup_helpers.c | 49 ++++++++++++++++++++
 tools/testing/selftests/bpf/cgroup_helpers.h |  1 +
 2 files changed, 50 insertions(+)

diff --git a/tools/testing/selftests/bpf/cgroup_helpers.c b/tools/testing/selftests/bpf/cgroup_helpers.c
index 63bfa72185be..d75bb875ef03 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.c
+++ b/tools/testing/selftests/bpf/cgroup_helpers.c
@@ -637,3 +637,52 @@ unsigned long long get_classid_cgroup_id(void)
 	format_classid_path(cgroup_workdir);
 	return get_cgroup_id_from_path(cgroup_workdir);
 }
+
+/**
+ * get_cgroup1_hierarchy_id - Retrieves the ID of a cgroup1 hierarchy from the cgroup1 name
+ * @cgrp_name: The cgroup1 name, which can be retrieved from /proc/self/cgroup.
+ */
+int get_cgroup1_hierarchy_id(const char *cgrp_name)
+{
+	char *c, *c2, *c3, *c4;
+	bool found = false;
+	char line[1024];
+	FILE *file;
+	int i, id;
+
+	if (!cgrp_name)
+		return -1;
+
+	file = fopen("/proc/self/cgroup", "r");
+	if (!file) {
+		log_err("fopen /proc/self/cgroup");
+		return -1;
+	}
+
+	while (fgets(line, 1024, file)) {
+		i = 0;
+		for (c = strtok_r(line, ":", &c2); c && i < 2; c = strtok_r(NULL, ":", &c2)) {
+			if (i == 0) {
+				id = strtol(c, NULL, 10);
+			} else if (i == 1) {
+				if (!strcmp(c, cgrp_name)) {
+					found = true;
+					break;
+				}
+
+				/* Multiple subsystems may share one single mount point */
+				for (c3 = strtok_r(c, ",", &c4); c3;
+				     c3 = strtok_r(NULL, ",", &c4)) {
+					if (!strcmp(c, cgrp_name)) {
+						found = true;
+						break;
+					}
+				}
+			}
+			i++;
+		}
+		if (found)
+			break;
+	}
+	return found ? id : -1;
+}
diff --git a/tools/testing/selftests/bpf/cgroup_helpers.h b/tools/testing/selftests/bpf/cgroup_helpers.h
index e71da4ef031b..a80c41734a26 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.h
+++ b/tools/testing/selftests/bpf/cgroup_helpers.h
@@ -20,6 +20,7 @@ int get_root_cgroup(void);
 int create_and_get_cgroup(const char *relative_path);
 void remove_cgroup(const char *relative_path);
 unsigned long long get_cgroup_id(const char *relative_path);
+int get_cgroup1_hierarchy_id(const char *cgrp_name);
 
 int join_cgroup(const char *relative_path);
 int join_root_cgroup(void);
-- 
2.30.1 (Apple Git-130)

[RFC PATCH bpf-next v2 9/9] selftests/bpf: Add selftests for cgroup1 hierarchy

[Yafang Shao]

Add selftests for cgroup1 hierarchy.
The result as follows,

  $ tools/testing/selftests/bpf/test_progs --name=cgroup1_hierarchy
  #36/1    cgroup1_hierarchy/test_cgroup1_hierarchy:OK
  #36/2    cgroup1_hierarchy/test_root_cgid:OK
  #36/3    cgroup1_hierarchy/test_invalid_level:OK
  #36/4    cgroup1_hierarchy/test_invalid_cgid:OK
  #36/5    cgroup1_hierarchy/test_invalid_hid:OK
  #36/6    cgroup1_hierarchy/test_invalid_cgrp_name:OK
  #36/7    cgroup1_hierarchy/test_invalid_cgrp_name2:OK
  #36/8    cgroup1_hierarchy/test_sleepable_prog:OK
  #36      cgroup1_hierarchy:OK
  Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 .../bpf/prog_tests/cgroup1_hierarchy.c        | 159 ++++++++++++++++++
 .../bpf/progs/test_cgroup1_hierarchy.c        |  73 ++++++++
 2 files changed, 232 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup1_hierarchy.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup1_hierarchy.c b/tools/testing/selftests/bpf/prog_tests/cgroup1_hierarchy.c
new file mode 100644
index 000000000000..4aafbc921254
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup1_hierarchy.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2023 Yafang Shao <laoar.shao@gmail.com> */
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <test_progs.h>
+#include "cgroup_helpers.h"
+#include "test_cgroup1_hierarchy.skel.h"
+
+static void bpf_cgroup1(struct test_cgroup1_hierarchy *skel)
+{
+	int err;
+
+	/* Attach LSM prog first */
+	skel->links.lsm_run = bpf_program__attach_lsm(skel->progs.lsm_run);
+	if (!ASSERT_OK_PTR(skel->links.lsm_run, "lsm_attach"))
+		return;
+
+	/* LSM prog will be triggered when attaching fentry */
+	skel->links.fentry_run = bpf_program__attach_trace(skel->progs.fentry_run);
+	ASSERT_NULL(skel->links.fentry_run, "fentry_attach_fail");
+
+	err = bpf_link__destroy(skel->links.lsm_run);
+	ASSERT_OK(err, "destroy_lsm");
+	skel->links.lsm_run = NULL;
+}
+
+static void bpf_cgroup1_sleepable(struct test_cgroup1_hierarchy *skel)
+{
+	int err;
+
+	/* Attach LSM prog first */
+	skel->links.lsm_s_run = bpf_program__attach_lsm(skel->progs.lsm_s_run);
+	if (!ASSERT_OK_PTR(skel->links.lsm_s_run, "lsm_attach"))
+		return;
+
+	/* LSM prog will be triggered when attaching fentry */
+	skel->links.fentry_run = bpf_program__attach_trace(skel->progs.fentry_run);
+	ASSERT_NULL(skel->links.fentry_run, "fentry_attach_fail");
+
+	err = bpf_link__destroy(skel->links.lsm_s_run);
+	ASSERT_OK(err, "destroy_lsm");
+	skel->links.lsm_s_run = NULL;
+}
+
+static void bpf_cgroup1_invalid_id(struct test_cgroup1_hierarchy *skel)
+{
+	int err;
+
+	/* Attach LSM prog first */
+	skel->links.lsm_run = bpf_program__attach_lsm(skel->progs.lsm_run);
+	if (!ASSERT_OK_PTR(skel->links.lsm_run, "lsm_attach"))
+		return;
+
+	/* LSM prog will be triggered when attaching fentry */
+	skel->links.fentry_run = bpf_program__attach_trace(skel->progs.fentry_run);
+	if (!ASSERT_OK_PTR(skel->links.fentry_run, "fentry_attach_success"))
+		goto cleanup;
+
+	err = bpf_link__destroy(skel->links.lsm_run);
+	ASSERT_OK(err, "destroy_lsm");
+	skel->links.lsm_run = NULL;
+
+cleanup:
+	err = bpf_link__destroy(skel->links.fentry_run);
+	ASSERT_OK(err, "destroy_fentry");
+	skel->links.fentry_run = NULL;
+}
+
+void test_cgroup1_hierarchy(void)
+{
+	struct test_cgroup1_hierarchy *skel;
+	__u64 current_cgid;
+	int hid, err;
+
+	skel = test_cgroup1_hierarchy__open();
+	if (!ASSERT_OK_PTR(skel, "open"))
+		return;
+
+	skel->bss->target_pid = getpid();
+
+	err = bpf_program__set_attach_target(skel->progs.fentry_run, 0, "bpf_fentry_test1");
+	if (!ASSERT_OK(err, "fentry_set_target"))
+		goto destroy;
+
+	err = test_cgroup1_hierarchy__load(skel);
+	if (!ASSERT_OK(err, "load"))
+		goto destroy;
+
+	/* Setup cgroup1 hierarchy */
+	err = setup_classid_environment();
+	if (!ASSERT_OK(err, "setup_classid_environment"))
+		goto destroy;
+
+	err = join_classid();
+	if (!ASSERT_OK(err, "join_cgroup1"))
+		goto cleanup;
+
+	current_cgid = get_classid_cgroup_id();
+	if (!ASSERT_GE(current_cgid, 0, "cgroup1 id"))
+		goto cleanup;
+
+	hid = get_cgroup1_hierarchy_id("net_cls");
+	if (!ASSERT_GE(hid, 0, "cgroup1 id"))
+		goto cleanup;
+	skel->bss->target_hid = hid;
+
+	if (test__start_subtest("test_cgroup1_hierarchy")) {
+		skel->bss->target_ancestor_cgid = current_cgid;
+		bpf_cgroup1(skel);
+	}
+
+	if (test__start_subtest("test_root_cgid")) {
+		skel->bss->target_ancestor_cgid = 1;
+		skel->bss->target_ancestor_level = 0;
+		bpf_cgroup1(skel);
+	}
+
+	if (test__start_subtest("test_invalid_level")) {
+		skel->bss->target_ancestor_cgid = 1;
+		skel->bss->target_ancestor_level = 1;
+		bpf_cgroup1_invalid_id(skel);
+	}
+
+	if (test__start_subtest("test_invalid_cgid")) {
+		skel->bss->target_ancestor_cgid = 0;
+		bpf_cgroup1_invalid_id(skel);
+	}
+
+	if (test__start_subtest("test_invalid_hid")) {
+		skel->bss->target_ancestor_cgid = 1;
+		skel->bss->target_ancestor_level = 0;
+		skel->bss->target_hid = -1;
+		bpf_cgroup1_invalid_id(skel);
+	}
+
+	if (test__start_subtest("test_invalid_cgrp_name")) {
+		skel->bss->target_hid = get_cgroup1_hierarchy_id("net_cl");
+		skel->bss->target_ancestor_cgid = current_cgid;
+		bpf_cgroup1_invalid_id(skel);
+	}
+
+	if (test__start_subtest("test_invalid_cgrp_name2")) {
+		skel->bss->target_hid = get_cgroup1_hierarchy_id("net_cls,");
+		skel->bss->target_ancestor_cgid = current_cgid;
+		bpf_cgroup1_invalid_id(skel);
+	}
+
+	if (test__start_subtest("test_sleepable_prog")) {
+		skel->bss->target_hid = hid;
+		skel->bss->target_ancestor_cgid = current_cgid;
+		bpf_cgroup1_sleepable(skel);
+	}
+
+cleanup:
+	cleanup_classid_environment();
+destroy:
+	test_cgroup1_hierarchy__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c b/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c
new file mode 100644
index 000000000000..ca9a631a6499
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0
+//#endif
+/* Copyright (C) 2023 Yafang Shao <laoar.shao@gmail.com> */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+
+__u32 target_ancestor_level;
+__u64 target_ancestor_cgid;
+int target_pid, target_hid;
+
+struct cgroup *
+bpf_task_get_cgroup1_within_hierarchy(struct task_struct *task, int hierarchy_id) __ksym;
+struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level) __ksym;
+void bpf_cgroup_release(struct cgroup *cgrp) __ksym;
+
+static int bpf_link_create_verify(int cmd)
+{
+	struct cgroup *cgrp, *ancestor;
+	struct task_struct *task;
+	int ret = 0;
+
+	if (cmd != BPF_LINK_CREATE)
+		return 0;
+
+	task = bpf_get_current_task_btf();
+
+	/* Then it can run in parallel with others */
+	if (task->pid != target_pid)
+		return 0;
+
+	cgrp = bpf_task_get_cgroup1_within_hierarchy(task, target_hid);
+	if (!cgrp)
+		return 0;
+
+	/* Refuse it if its cgid or its ancestor's cgid is the target cgid */
+	if (cgrp->kn->id == target_ancestor_cgid)
+		ret = -1;
+
+	ancestor = bpf_cgroup_ancestor(cgrp, target_ancestor_level);
+	if (!ancestor)
+		goto out;
+
+	if (ancestor->kn->id == target_ancestor_cgid)
+		ret = -1;
+	bpf_cgroup_release(ancestor);
+
+out:
+	bpf_cgroup_release(cgrp);
+	return ret;
+}
+
+SEC("lsm/bpf")
+int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size)
+{
+	return bpf_link_create_verify(cmd);
+}
+
+SEC("lsm.s/bpf")
+int BPF_PROG(lsm_s_run, int cmd, union bpf_attr *attr, unsigned int size)
+{
+	return bpf_link_create_verify(cmd);
+}
+
+SEC("fentry")
+int BPF_PROG(fentry_run)
+{
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.30.1 (Apple Git-130)