[PATCH v3 bpf-next 00/11] bpf, cgroup: Add BPF support for cgroup1 hierarchy

[PATCH v3 bpf-next 00/11] 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[0], 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
  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`. As pointed out by Michal, we can further
replace cgroup_mutex with the RCU read lock in other paths like
cgroup_path_ns(), but it necessitates some refactoring, so I prefer to
address it in a separate patchset.

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

Changes:
- RFC v2 -> v3:
  - Use [bpf_]task_get_cgroup1 instead (Tejun)
  - Skip the unmounted cgroup in /proc/self/cgroup (Tejun)
  - Fix lockdep_is_held() in list_for_each_entry_rcu()
    (Tejun, oliver.sang@intel.com)
  - Drop the warning in __cset_cgroup_from_root()(Tejun) 
  - Fix mismatched comments (Tejun, lkp@intel.com)
  - Add a explicit warning in current_cgns_cgroup_from_root (Michal)
- 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 (11):
  cgroup: Remove unnecessary list_empty()
  cgroup: Make operations on the cgroup root_list RCU safe
  cgroup: Eliminate the need for cgroup_mutex in proc_cgroup_show()
  cgroup: Add annotation for holding namespace_sem in
    current_cgns_cgroup_from_root()
  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                             |   4 +-
 kernel/bpf/helpers.c                               |  20 +++
 kernel/cgroup/cgroup-internal.h                    |   4 +-
 kernel/cgroup/cgroup-v1.c                          |  33 +++++
 kernel/cgroup/cgroup.c                             |  45 ++++--
 tools/testing/selftests/bpf/cgroup_helpers.c       | 113 ++++++++++++---
 tools/testing/selftests/bpf/cgroup_helpers.h       |   4 +-
 .../selftests/bpf/prog_tests/cgroup1_hierarchy.c   | 159 +++++++++++++++++++++
 .../testing/selftests/bpf/prog_tests/cgroup_v1v2.c |   2 +-
 .../selftests/bpf/progs/test_cgroup1_hierarchy.c   |  72 ++++++++++
 11 files changed, 420 insertions(+), 37 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

-- 
1.8.3.1

[PATCH v3 bpf-next 01/11] cgroup: Remove unnecessary list_empty()

[Yafang Shao]

The root hasn't been removed from the root_list, so the list can't be NULL.
However, if it had been removed, attempting to destroy it once more is not
possible. Let's replace this with WARN_ON_ONCE() for clarity.

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

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 1fb7f56..3053d42 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1345,10 +1345,9 @@ 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);
-		cgroup_root_count--;
-	}
+	WARN_ON_ONCE(list_empty(&root->root_list));
+	list_del(&root->root_list);
+	cgroup_root_count--;
 
 	cgroup_favor_dynmods(root, false);
 	cgroup_exit_root_id(root);
-- 
1.8.3.1

Re: [PATCH v3 bpf-next 01/11] cgroup: Remove unnecessary list_empty()

[Tejun Heo]

On Sun, Oct 29, 2023 at 06:14:28AM +0000, Yafang Shao wrote:
> The root hasn't been removed from the root_list, so the list can't be NULL.
> However, if it had been removed, attempting to destroy it once more is not
> possible. Let's replace this with WARN_ON_ONCE() for clarity.
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>

Acked-by: Tejun Heo <tj@kernel.org>

-- 
tejun

[PATCH v3 bpf-next 02/11] 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          | 23 ++++++++++++++++-------
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index f1b3151..8505eea 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 c56071f..5e17f01 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -170,7 +170,8 @@ struct cgroup_mgctx {
 
 /* 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 3053d42..28b8ccc 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);
 
 	WARN_ON_ONCE(list_empty(&root->root_list));
-	list_del(&root->root_list);
+	list_del_rcu(&root->root_list);
 	cgroup_root_count--;
 
 	cgroup_favor_dynmods(root, false);
@@ -1385,7 +1385,15 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
 		}
 	}
 
-	BUG_ON(!res_cgroup);
+	/*
+	 * If cgroup_mutex is not held, the cgrp_cset_link will be 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 can't be NULL.
+	 * If we don't hold cgroup_mutex in the caller, we must do the NULL
+	 * check.
+	 */
 	return res_cgroup;
 }
 
@@ -1444,7 +1452,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);
@@ -1452,7 +1459,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)
@@ -2013,7 +2022,7 @@ void init_cgroup_root(struct cgroup_fs_context *ctx)
 	struct cgroup_root *root = ctx->root;
 	struct cgroup *cgrp = &root->cgrp;
 
-	INIT_LIST_HEAD(&root->root_list);
+	INIT_LIST_HEAD_RCU(&root->root_list);
 	atomic_set(&root->nr_cgrps, 1);
 	cgrp->root = root;
 	init_cgroup_housekeeping(cgrp);
@@ -2096,7 +2105,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++;
 
 	/*
-- 
1.8.3.1

Re: [PATCH v3 bpf-next 02/11] cgroup: Make operations on the cgroup root_list RCU safe

[Tejun Heo]

On Sun, Oct 29, 2023 at 06:14:29AM +0000, Yafang Shao wrote:
> 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>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

[PATCH v3 bpf-next 03/11] 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.

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

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 28b8ccc..cc6a6d9 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6261,7 +6261,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) {
@@ -6272,6 +6272,11 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 		if (root == &cgrp_dfl_root && !READ_ONCE(cgrp_dfl_visible))
 			continue;
 
+		cgrp = task_cgroup_from_root(tsk, root);
+		/* The root has already been unmounted. */
+		if (!cgrp)
+			continue;
+
 		seq_printf(m, "%d:", root->hierarchy_id);
 		if (root != &cgrp_dfl_root)
 			for_each_subsys(ss, ssid)
@@ -6282,9 +6287,6 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 			seq_printf(m, "%sname=%s", count ? "," : "",
 				   root->name);
 		seq_putc(m, ':');
-
-		cgrp = task_cgroup_from_root(tsk, root);
-
 		/*
 		 * On traditional hierarchies, all zombie tasks show up as
 		 * belonging to the root cgroup.  On the default hierarchy,
@@ -6316,7 +6318,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;
-- 
1.8.3.1

Re: [PATCH v3 bpf-next 03/11] cgroup: Eliminate the need for cgroup_mutex in proc_cgroup_show()

[Tejun Heo]

On Sun, Oct 29, 2023 at 06:14:30AM +0000, Yafang Shao wrote:
> 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.
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

[PATCH v3 bpf-next 04/11] cgroup: Add annotation for holding namespace_sem in current_cgns_cgroup_from_root()

[Yafang Shao]

When I initially examined the function current_cgns_cgroup_from_root(), I
was perplexed by its lack of holding cgroup_mutex. However, after Michal
explained the reason[0] to me, I realized that it already holds the
namespace_sem. I believe this intricacy could also confuse others, so it
would be advisable to include an annotation for clarification.

After we replace the cgroup_mutex with RCU read lock, if current doesn't
hold the namespace_sem, the root cgroup will be NULL. So let's add a
WARN_ON_ONCE() for it.

[0]. https://lore.kernel.org/bpf/afdnpo3jz2ic2ampud7swd6so5carkilts2mkygcaw67vbw6yh@5b5mncf7qyet

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: Michal Koutný <mkoutny@suse.com>
---
 kernel/cgroup/cgroup.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index cc6a6d9..baba4b1 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1416,6 +1416,11 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
 
 	rcu_read_unlock();
 
+	/*
+	 * The namespace_sem is held by current, so the root cgroup can't
+	 * be umounted. Therefore, we can ensure that the res is non-NULL.
+	 */
+	WARN_ON_ONCE(!res);
 	return res;
 }
 
-- 
1.8.3.1

Re: [PATCH v3 bpf-next 04/11] cgroup: Add annotation for holding namespace_sem in current_cgns_cgroup_from_root()

[Tejun Heo]

On Sun, Oct 29, 2023 at 06:14:31AM +0000, Yafang Shao wrote:
> When I initially examined the function current_cgns_cgroup_from_root(), I
> was perplexed by its lack of holding cgroup_mutex. However, after Michal
> explained the reason[0] to me, I realized that it already holds the
> namespace_sem. I believe this intricacy could also confuse others, so it
> would be advisable to include an annotation for clarification.
> 
> After we replace the cgroup_mutex with RCU read lock, if current doesn't
> hold the namespace_sem, the root cgroup will be NULL. So let's add a
> WARN_ON_ONCE() for it.
> 
> [0]. https://lore.kernel.org/bpf/afdnpo3jz2ic2ampud7swd6so5carkilts2mkygcaw67vbw6yh@5b5mncf7qyet
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Cc: Michal Koutný <mkoutny@suse.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

[PATCH v3 bpf-next 05/11] cgroup: Add a new helper for cgroup1 hierarchy

[Yafang Shao]

A new helper is added for cgroup1 hierarchy:

- task_get_cgroup1
  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.

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

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index b307013..e063e4c 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>
@@ -388,7 +389,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 +855,6 @@ static inline void cgroup_bpf_put(struct cgroup *cgrp) {}
 
 #endif /* CONFIG_CGROUP_BPF */
 
+struct cgroup *task_get_cgroup1(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 5e17f01..520b90d 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 c487ffe..f41767f 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_get_cgroup1 - 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(struct task_struct *tsk, int hierarchy_id)
+{
+	struct cgroup *cgrp = ERR_PTR(-ENOENT);
+	struct cgroup_root *root;
+
+	rcu_read_lock();
+	for_each_root(root) {
+		/* 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)
 {
 	/*
-- 
1.8.3.1

Re: [PATCH v3 bpf-next 05/11] cgroup: Add a new helper for cgroup1 hierarchy

[Hou Tao]

Hi,

On 10/29/2023 2:14 PM, Yafang Shao wrote:
> A new helper is added for cgroup1 hierarchy:
>
> - task_get_cgroup1
>   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.
>
> Suggested-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  include/linux/cgroup.h          |  4 +++-
>  kernel/cgroup/cgroup-internal.h |  1 -
>  kernel/cgroup/cgroup-v1.c       | 33 +++++++++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index b307013..e063e4c 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>
> @@ -388,7 +389,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 +855,6 @@ static inline void cgroup_bpf_put(struct cgroup *cgrp) {}
>  
>  #endif /* CONFIG_CGROUP_BPF */
>  
> +struct cgroup *task_get_cgroup1(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 5e17f01..520b90d 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 c487ffe..f41767f 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_get_cgroup1 - 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(struct task_struct *tsk, int hierarchy_id)
> +{
> +	struct cgroup *cgrp = ERR_PTR(-ENOENT);
> +	struct cgroup_root *root;
> +
> +	rcu_read_lock();
> +	for_each_root(root) {
> +		/* cgroup1 only*/
> +		if (root == &cgrp_dfl_root)
> +			continue;
> +		if (root->hierarchy_id != hierarchy_id)
> +			continue;
> +		spin_lock_irq(&css_set_lock);

Considering that the kfunc may be called under IRQ context, should we
use spin_lock_irqsave instead ?
> +		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)
>  {
>  	/*

Re: [PATCH v3 bpf-next 05/11] cgroup: Add a new helper for cgroup1 hierarchy

[Tejun Heo]

Hello,

On Sun, Oct 29, 2023 at 06:14:32AM +0000, Yafang Shao wrote:
> +/**
> + * task_get_cgroup1 - 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(struct task_struct *tsk, int hierarchy_id)
> +{
> +	struct cgroup *cgrp = ERR_PTR(-ENOENT);
> +	struct cgroup_root *root;
> +
> +	rcu_read_lock();
> +	for_each_root(root) {
> +		/* 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);

As Hou suggested, please use irqsave/restore. Other than that, looks fine to
me.

Thanks.

-- 
tejun

Re: [PATCH v3 bpf-next 05/11] cgroup: Add a new helper for cgroup1 hierarchy

[Tejun Heo]

Hello,

The following is the version updated to use irqsave/restore applied to
cgroup/for-6.8-bpf.

Thanks.
--- 8< ---
From aecd408b7e50742868b3305c24325a89024e2a30 Mon Sep 17 00:00:00 2001
From: Yafang Shao <laoar.shao@gmail.com>
Date: Sun, 29 Oct 2023 06:14:32 +0000
Subject: [PATCH] cgroup: Add a new helper for cgroup1 hierarchy

A new helper is added for cgroup1 hierarchy:

- task_get_cgroup1
  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.

tj: Use irsqsave/restore as suggested by Hou Tao <houtao@huaweicloud.com>.

Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: Hou Tao <houtao@huaweicloud.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 include/linux/cgroup.h          |  4 +++-
 kernel/cgroup/cgroup-internal.h |  1 -
 kernel/cgroup/cgroup-v1.c       | 34 +++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 0ef0af66080e..34aaf0e87def 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -69,6 +69,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>
@@ -386,7 +387,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() ||				\
@@ -853,4 +853,6 @@ static inline void cgroup_bpf_put(struct cgroup *cgrp) {}
 
 #endif /* CONFIG_CGROUP_BPF */
 
+struct cgroup *task_get_cgroup1(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 5e17f01ced9f..520b90dd97ec 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 76db6c67e39a..04d11a7dd95f 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -1262,6 +1262,40 @@ int cgroup1_get_tree(struct fs_context *fc)
 	return ret;
 }
 
+/**
+ * task_get_cgroup1 - 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(struct task_struct *tsk, int hierarchy_id)
+{
+	struct cgroup *cgrp = ERR_PTR(-ENOENT);
+	struct cgroup_root *root;
+	unsigned long flags;
+
+	rcu_read_lock();
+	for_each_root(root) {
+		/* cgroup1 only*/
+		if (root == &cgrp_dfl_root)
+			continue;
+		if (root->hierarchy_id != hierarchy_id)
+			continue;
+		spin_lock_irqsave(&css_set_lock, flags);
+		cgrp = task_cgroup_from_root(tsk, root);
+		if (!cgrp || !cgroup_tryget(cgrp))
+			cgrp = ERR_PTR(-ENOENT);
+		spin_unlock_irqrestore(&css_set_lock, flags);
+		break;
+	}
+	rcu_read_unlock();
+	return cgrp;
+}
+
 static int __init cgroup1_wq_init(void)
 {
 	/*
-- 
2.42.0

Re: [PATCH v3 bpf-next 05/11] cgroup: Add a new helper for cgroup1 hierarchy

[Yafang Shao]

On Fri, Nov 10, 2023 at 7:30 AM Tejun Heo <tj@kernel.org> wrote:
>
> Hello,
>
> The following is the version updated to use irqsave/restore applied to
> cgroup/for-6.8-bpf.
>

Thanks for your update and also thanks for the suggestion from Hou.

> [...]

-- 
Regards
Yafang

[PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy

[Yafang Shao]

A new kfunc is added to acquire cgroup1 of a task:

- bpf_task_get_cgroup1
  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.

Suggested-by: Tejun Heo <tj@kernel.org>
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 61f51de..f4cdeae 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2222,6 +2222,25 @@ __bpf_kfunc long bpf_task_under_cgroup(struct task_struct *task,
 	rcu_read_unlock();
 	return ret;
 }
+
+/**
+ * bpf_task_get_cgroup1 - 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(struct task_struct *task, int hierarchy_id)
+{
+	struct cgroup *cgrp = task_get_cgroup1(task, hierarchy_id);
+
+	if (IS_ERR(cgrp))
+		return NULL;
+	return cgrp;
+}
 #endif /* CONFIG_CGROUPS */
 
 /**
@@ -2528,6 +2547,7 @@ __bpf_kfunc void bpf_throw(u64 cookie)
 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, 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)
-- 
1.8.3.1

Re: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy

[kernel test robot]

Hi Yafang,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-Remove-unnecessary-list_empty/20231029-143457
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20231029061438.4215-7-laoar.shao%40gmail.com
patch subject: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy
config: i386-randconfig-013-20231030 (https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-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/202310301605.CGFI0aSW-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/bpf/helpers.c:1893:19: warning: no previous declaration for 'bpf_obj_new_impl' [-Wmissing-declarations]
    __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
                      ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1907:19: warning: no previous declaration for 'bpf_percpu_obj_new_impl' [-Wmissing-declarations]
    __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, void *meta__ign)
                      ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1941:18: warning: no previous declaration for 'bpf_obj_drop_impl' [-Wmissing-declarations]
    __bpf_kfunc void bpf_obj_drop_impl(void *p__alloc, void *meta__ign)
                     ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1949:18: warning: no previous declaration for 'bpf_percpu_obj_drop_impl' [-Wmissing-declarations]
    __bpf_kfunc void bpf_percpu_obj_drop_impl(void *p__alloc, void *meta__ign)
                     ^~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1955:19: warning: no previous declaration for 'bpf_refcount_acquire_impl' [-Wmissing-declarations]
    __bpf_kfunc void *bpf_refcount_acquire_impl(void *p__refcounted_kptr, void *meta__ign)
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2000:17: warning: no previous declaration for 'bpf_list_push_front_impl' [-Wmissing-declarations]
    __bpf_kfunc int bpf_list_push_front_impl(struct bpf_list_head *head,
                    ^~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2010:17: warning: no previous declaration for 'bpf_list_push_back_impl' [-Wmissing-declarations]
    __bpf_kfunc int bpf_list_push_back_impl(struct bpf_list_head *head,
                    ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2043:35: warning: no previous declaration for 'bpf_list_pop_front' [-Wmissing-declarations]
    __bpf_kfunc struct bpf_list_node *bpf_list_pop_front(struct bpf_list_head *head)
                                      ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2048:35: warning: no previous declaration for 'bpf_list_pop_back' [-Wmissing-declarations]
    __bpf_kfunc struct bpf_list_node *bpf_list_pop_back(struct bpf_list_head *head)
                                      ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2053:33: warning: no previous declaration for 'bpf_rbtree_remove' [-Wmissing-declarations]
    __bpf_kfunc struct bpf_rb_node *bpf_rbtree_remove(struct bpf_rb_root *root,
                                    ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2109:17: warning: no previous declaration for 'bpf_rbtree_add_impl' [-Wmissing-declarations]
    __bpf_kfunc int bpf_rbtree_add_impl(struct bpf_rb_root *root, struct bpf_rb_node *node,
                    ^~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2119:33: warning: no previous declaration for 'bpf_rbtree_first' [-Wmissing-declarations]
    __bpf_kfunc struct bpf_rb_node *bpf_rbtree_first(struct bpf_rb_root *root)
                                    ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2132:33: warning: no previous declaration for 'bpf_task_acquire' [-Wmissing-declarations]
    __bpf_kfunc struct task_struct *bpf_task_acquire(struct task_struct *p)
                                    ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2143:18: warning: no previous declaration for 'bpf_task_release' [-Wmissing-declarations]
    __bpf_kfunc void bpf_task_release(struct task_struct *p)
                     ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2155:28: warning: no previous declaration for 'bpf_cgroup_acquire' [-Wmissing-declarations]
    __bpf_kfunc struct cgroup *bpf_cgroup_acquire(struct cgroup *cgrp)
                               ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2167:18: warning: no previous declaration for 'bpf_cgroup_release' [-Wmissing-declarations]
    __bpf_kfunc void bpf_cgroup_release(struct cgroup *cgrp)
                     ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2179:28: warning: no previous declaration for 'bpf_cgroup_ancestor' [-Wmissing-declarations]
    __bpf_kfunc struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level)
                               ^~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2199:28: warning: no previous declaration for 'bpf_cgroup_from_id' [-Wmissing-declarations]
    __bpf_kfunc struct cgroup *bpf_cgroup_from_id(u64 cgid)
                               ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2219:18: warning: no previous declaration for 'bpf_task_under_cgroup' [-Wmissing-declarations]
    __bpf_kfunc long bpf_task_under_cgroup(struct task_struct *task,
                     ^~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/helpers.c:2240:1: warning: no previous declaration for 'bpf_task_get_cgroup1' [-Wmissing-declarations]
    bpf_task_get_cgroup1(struct task_struct *task, int hierarchy_id)
    ^~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2256:33: warning: no previous declaration for 'bpf_task_from_pid' [-Wmissing-declarations]
    __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
                                    ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2297:19: warning: no previous declaration for 'bpf_dynptr_slice' [-Wmissing-declarations]
    __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset,
                      ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2381:19: warning: no previous declaration for 'bpf_dynptr_slice_rdwr' [-Wmissing-declarations]
    __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset,
                      ^~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2412:17: warning: no previous declaration for 'bpf_dynptr_adjust' [-Wmissing-declarations]
    __bpf_kfunc int bpf_dynptr_adjust(struct bpf_dynptr_kern *ptr, u32 start, u32 end)
                    ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2430:18: warning: no previous declaration for 'bpf_dynptr_is_null' [-Wmissing-declarations]
    __bpf_kfunc bool bpf_dynptr_is_null(struct bpf_dynptr_kern *ptr)
                     ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2435:18: warning: no previous declaration for 'bpf_dynptr_is_rdonly' [-Wmissing-declarations]
    __bpf_kfunc bool bpf_dynptr_is_rdonly(struct bpf_dynptr_kern *ptr)
                     ^~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2443:19: warning: no previous declaration for 'bpf_dynptr_size' [-Wmissing-declarations]
    __bpf_kfunc __u32 bpf_dynptr_size(const struct bpf_dynptr_kern *ptr)
                      ^~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2451:17: warning: no previous declaration for 'bpf_dynptr_clone' [-Wmissing-declarations]
    __bpf_kfunc int bpf_dynptr_clone(struct bpf_dynptr_kern *ptr,
                    ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2464:19: warning: no previous declaration for 'bpf_cast_to_kern_ctx' [-Wmissing-declarations]
    __bpf_kfunc void *bpf_cast_to_kern_ctx(void *obj)
                      ^~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2469:19: warning: no previous declaration for 'bpf_rdonly_cast' [-Wmissing-declarations]
    __bpf_kfunc void *bpf_rdonly_cast(void *obj__ign, u32 btf_id__k)
                      ^~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2474:18: warning: no previous declaration for 'bpf_rcu_read_lock' [-Wmissing-declarations]
    __bpf_kfunc void bpf_rcu_read_lock(void)
                     ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2479:18: warning: no previous declaration for 'bpf_rcu_read_unlock' [-Wmissing-declarations]
    __bpf_kfunc void bpf_rcu_read_unlock(void)
                     ^~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2508:18: warning: no previous declaration for 'bpf_throw' [-Wmissing-declarations]
    __bpf_kfunc void bpf_throw(u64 cookie)
                     ^~~~~~~~~


vim +/bpf_task_get_cgroup1 +2240 kernel/bpf/helpers.c

  2229	
  2230	/**
  2231	 * bpf_task_get_cgroup1 - Acquires the associated cgroup of a task within a
  2232	 * specific cgroup1 hierarchy. The cgroup1 hierarchy is identified by its
  2233	 * hierarchy ID.
  2234	 * @task: The target task
  2235	 * @hierarchy_id: The ID of a cgroup1 hierarchy
  2236	 *
  2237	 * On success, the cgroup is returen. On failure, NULL is returned.
  2238	 */
  2239	__bpf_kfunc struct cgroup *
> 2240	bpf_task_get_cgroup1(struct task_struct *task, int hierarchy_id)
  2241	{
  2242		struct cgroup *cgrp = task_get_cgroup1(task, hierarchy_id);
  2243	
  2244		if (IS_ERR(cgrp))
  2245			return NULL;
  2246		return cgrp;
  2247	}
  2248	#endif /* CONFIG_CGROUPS */
  2249	

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

Re: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy

[Yafang Shao]

On Mon, Oct 30, 2023 at 5:01 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Yafang,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on bpf-next/master]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-Remove-unnecessary-list_empty/20231029-143457
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> patch link:    https://lore.kernel.org/r/20231029061438.4215-7-laoar.shao%40gmail.com
> patch subject: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy
> config: i386-randconfig-013-20231030 (https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-lkp@intel.com/config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-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/202310301605.CGFI0aSW-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
>    kernel/bpf/helpers.c:1893:19: warning: no previous declaration for 'bpf_obj_new_impl' [-Wmissing-declarations]

-Wmissing-declarations is a known issue and somebody is working on it, right?

-- 
Regards
Yafang

Re: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy

[Jiri Olsa]

On Mon, Oct 30, 2023 at 07:35:25PM +0800, Yafang Shao wrote:
> On Mon, Oct 30, 2023 at 5:01 PM kernel test robot <lkp@intel.com> wrote:
> >
> > Hi Yafang,
> >
> > kernel test robot noticed the following build warnings:
> >
> > [auto build test WARNING on bpf-next/master]
> >
> > url:    https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-Remove-unnecessary-list_empty/20231029-143457
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> > patch link:    https://lore.kernel.org/r/20231029061438.4215-7-laoar.shao%40gmail.com
> > patch subject: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy
> > config: i386-randconfig-013-20231030 (https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-lkp@intel.com/config)
> > compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-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/202310301605.CGFI0aSW-lkp@intel.com/
> >
> > All warnings (new ones prefixed by >>):
> >
> >    kernel/bpf/helpers.c:1893:19: warning: no previous declaration for 'bpf_obj_new_impl' [-Wmissing-declarations]
> 
> -Wmissing-declarations is a known issue and somebody is working on it, right?

there's this post [1] from Dave, but seems it never landed

jirka

[1] https://lore.kernel.org/bpf/20230816150634.1162838-1-void@manifault.com/

Re: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy

[Yafang Shao]

On Mon, Oct 30, 2023 at 9:59 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Oct 30, 2023 at 07:35:25PM +0800, Yafang Shao wrote:
> > On Mon, Oct 30, 2023 at 5:01 PM kernel test robot <lkp@intel.com> wrote:
> > >
> > > Hi Yafang,
> > >
> > > kernel test robot noticed the following build warnings:
> > >
> > > [auto build test WARNING on bpf-next/master]
> > >
> > > url:    https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-Remove-unnecessary-list_empty/20231029-143457
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> > > patch link:    https://lore.kernel.org/r/20231029061438.4215-7-laoar.shao%40gmail.com
> > > patch subject: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy
> > > config: i386-randconfig-013-20231030 (https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-lkp@intel.com/config)
> > > compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-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/202310301605.CGFI0aSW-lkp@intel.com/
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > >    kernel/bpf/helpers.c:1893:19: warning: no previous declaration for 'bpf_obj_new_impl' [-Wmissing-declarations]
> >
> > -Wmissing-declarations is a known issue and somebody is working on it, right?
>
> there's this post [1] from Dave, but seems it never landed
>
> jirka
>
> [1] https://lore.kernel.org/bpf/20230816150634.1162838-1-void@manifault.com/

Thanks for your information.

David, I'd appreciate it if you could share an update on its current status

-- 
Regards
Yafang

Re: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy

[David Marchevsky]

On 10/30/23 10:40 PM, Yafang Shao wrote:
> On Mon, Oct 30, 2023 at 9:59 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>>
>> On Mon, Oct 30, 2023 at 07:35:25PM +0800, Yafang Shao wrote:
>>> On Mon, Oct 30, 2023 at 5:01 PM kernel test robot <lkp@intel.com> wrote:
>>>>
>>>> Hi Yafang,
>>>>
>>>> kernel test robot noticed the following build warnings:
>>>>
>>>> [auto build test WARNING on bpf-next/master]
>>>>
>>>> url:    https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-Remove-unnecessary-list_empty/20231029-143457
>>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
>>>> patch link:    https://lore.kernel.org/r/20231029061438.4215-7-laoar.shao%40gmail.com
>>>> patch subject: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy
>>>> config: i386-randconfig-013-20231030 (https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-lkp@intel.com/config__;!!Bt8RZUm9aw!4feH_paRfc92c6DKnIgcbDciELDAOzSoIr66fN3591gkU9ddriq2cqyFm47OezGzvLvXd5Ep4R9ZYmtxrW9dqg$ )
>>>> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
>>>> reproduce (this is a W=1 build): (https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20231030/202310301605.CGFI0aSW-lkp@intel.com/reproduce__;!!Bt8RZUm9aw!4feH_paRfc92c6DKnIgcbDciELDAOzSoIr66fN3591gkU9ddriq2cqyFm47OezGzvLvXd5Ep4R9ZYmsRR5GsUA$ )
>>>>
>>>> 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/202310301605.CGFI0aSW-lkp@intel.com/
>>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>>    kernel/bpf/helpers.c:1893:19: warning: no previous declaration for 'bpf_obj_new_impl' [-Wmissing-declarations]
>>>
>>> -Wmissing-declarations is a known issue and somebody is working on it, right?
>>
>> there's this post [1] from Dave, but seems it never landed
>>
>> jirka
>>
>> [1] https://lore.kernel.org/bpf/20230816150634.1162838-1-void@manifault.com/
> 
> Thanks for your information.
> 
> David, I'd appreciate it if you could share an update on its current status
> 

Hi Yafang,

I had a similar patch in a recent series that I was asked to
split out and submit separately. Did so today after seeing this thread: https://lore.kernel.org/bpf/20231030210638.2415306-1-davemarchevsky@fb.com/
Sorry, should've CC'd you as well as Jiri on that patch.

Re: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy

[kernel test robot]

Hi Yafang,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-Remove-unnecessary-list_empty/20231029-143457
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20231029061438.4215-7-laoar.shao%40gmail.com
patch subject: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy
config: x86_64-randconfig-004-20231103 (https://download.01.org/0day-ci/archive/20231103/202311031651.A7crZEur-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231103/202311031651.A7crZEur-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/202311031651.A7crZEur-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/bpf/helpers.c:1893:19: warning: no previous prototype for 'bpf_obj_new_impl' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
                      ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1907:19: warning: no previous prototype for 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, void *meta__ign)
                      ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1941:18: warning: no previous prototype for 'bpf_obj_drop_impl' [-Wmissing-prototypes]
    __bpf_kfunc void bpf_obj_drop_impl(void *p__alloc, void *meta__ign)
                     ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1949:18: warning: no previous prototype for 'bpf_percpu_obj_drop_impl' [-Wmissing-prototypes]
    __bpf_kfunc void bpf_percpu_obj_drop_impl(void *p__alloc, void *meta__ign)
                     ^~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1955:19: warning: no previous prototype for 'bpf_refcount_acquire_impl' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_refcount_acquire_impl(void *p__refcounted_kptr, void *meta__ign)
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2000:17: warning: no previous prototype for 'bpf_list_push_front_impl' [-Wmissing-prototypes]
    __bpf_kfunc int bpf_list_push_front_impl(struct bpf_list_head *head,
                    ^~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2010:17: warning: no previous prototype for 'bpf_list_push_back_impl' [-Wmissing-prototypes]
    __bpf_kfunc int bpf_list_push_back_impl(struct bpf_list_head *head,
                    ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2043:35: warning: no previous prototype for 'bpf_list_pop_front' [-Wmissing-prototypes]
    __bpf_kfunc struct bpf_list_node *bpf_list_pop_front(struct bpf_list_head *head)
                                      ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2048:35: warning: no previous prototype for 'bpf_list_pop_back' [-Wmissing-prototypes]
    __bpf_kfunc struct bpf_list_node *bpf_list_pop_back(struct bpf_list_head *head)
                                      ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2053:33: warning: no previous prototype for 'bpf_rbtree_remove' [-Wmissing-prototypes]
    __bpf_kfunc struct bpf_rb_node *bpf_rbtree_remove(struct bpf_rb_root *root,
                                    ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2109:17: warning: no previous prototype for 'bpf_rbtree_add_impl' [-Wmissing-prototypes]
    __bpf_kfunc int bpf_rbtree_add_impl(struct bpf_rb_root *root, struct bpf_rb_node *node,
                    ^~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2119:33: warning: no previous prototype for 'bpf_rbtree_first' [-Wmissing-prototypes]
    __bpf_kfunc struct bpf_rb_node *bpf_rbtree_first(struct bpf_rb_root *root)
                                    ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2132:33: warning: no previous prototype for 'bpf_task_acquire' [-Wmissing-prototypes]
    __bpf_kfunc struct task_struct *bpf_task_acquire(struct task_struct *p)
                                    ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2143:18: warning: no previous prototype for 'bpf_task_release' [-Wmissing-prototypes]
    __bpf_kfunc void bpf_task_release(struct task_struct *p)
                     ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2155:28: warning: no previous prototype for 'bpf_cgroup_acquire' [-Wmissing-prototypes]
    __bpf_kfunc struct cgroup *bpf_cgroup_acquire(struct cgroup *cgrp)
                               ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2167:18: warning: no previous prototype for 'bpf_cgroup_release' [-Wmissing-prototypes]
    __bpf_kfunc void bpf_cgroup_release(struct cgroup *cgrp)
                     ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2179:28: warning: no previous prototype for 'bpf_cgroup_ancestor' [-Wmissing-prototypes]
    __bpf_kfunc struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level)
                               ^~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2199:28: warning: no previous prototype for 'bpf_cgroup_from_id' [-Wmissing-prototypes]
    __bpf_kfunc struct cgroup *bpf_cgroup_from_id(u64 cgid)
                               ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2219:18: warning: no previous prototype for 'bpf_task_under_cgroup' [-Wmissing-prototypes]
    __bpf_kfunc long bpf_task_under_cgroup(struct task_struct *task,
                     ^~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/helpers.c:2240:1: warning: no previous prototype for 'bpf_task_get_cgroup1' [-Wmissing-prototypes]
    bpf_task_get_cgroup1(struct task_struct *task, int hierarchy_id)
    ^~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2256:33: warning: no previous prototype for 'bpf_task_from_pid' [-Wmissing-prototypes]
    __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
                                    ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2297:19: warning: no previous prototype for 'bpf_dynptr_slice' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset,
                      ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2381:19: warning: no previous prototype for 'bpf_dynptr_slice_rdwr' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset,
                      ^~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2412:17: warning: no previous prototype for 'bpf_dynptr_adjust' [-Wmissing-prototypes]
    __bpf_kfunc int bpf_dynptr_adjust(struct bpf_dynptr_kern *ptr, u32 start, u32 end)
                    ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2430:18: warning: no previous prototype for 'bpf_dynptr_is_null' [-Wmissing-prototypes]
    __bpf_kfunc bool bpf_dynptr_is_null(struct bpf_dynptr_kern *ptr)
                     ^~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2435:18: warning: no previous prototype for 'bpf_dynptr_is_rdonly' [-Wmissing-prototypes]
    __bpf_kfunc bool bpf_dynptr_is_rdonly(struct bpf_dynptr_kern *ptr)
                     ^~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2443:19: warning: no previous prototype for 'bpf_dynptr_size' [-Wmissing-prototypes]
    __bpf_kfunc __u32 bpf_dynptr_size(const struct bpf_dynptr_kern *ptr)
                      ^~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2451:17: warning: no previous prototype for 'bpf_dynptr_clone' [-Wmissing-prototypes]
    __bpf_kfunc int bpf_dynptr_clone(struct bpf_dynptr_kern *ptr,
                    ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2464:19: warning: no previous prototype for 'bpf_cast_to_kern_ctx' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_cast_to_kern_ctx(void *obj)
                      ^~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2469:19: warning: no previous prototype for 'bpf_rdonly_cast' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_rdonly_cast(void *obj__ign, u32 btf_id__k)
                      ^~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2474:18: warning: no previous prototype for 'bpf_rcu_read_lock' [-Wmissing-prototypes]
    __bpf_kfunc void bpf_rcu_read_lock(void)
                     ^~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2479:18: warning: no previous prototype for 'bpf_rcu_read_unlock' [-Wmissing-prototypes]
    __bpf_kfunc void bpf_rcu_read_unlock(void)
                     ^~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:2508:18: warning: no previous prototype for 'bpf_throw' [-Wmissing-prototypes]
    __bpf_kfunc void bpf_throw(u64 cookie)
                     ^~~~~~~~~
   cc1: warning: unrecognized command line option '-Wno-attribute-alias'


vim +/bpf_task_get_cgroup1 +2240 kernel/bpf/helpers.c

  2229	
  2230	/**
  2231	 * bpf_task_get_cgroup1 - Acquires the associated cgroup of a task within a
  2232	 * specific cgroup1 hierarchy. The cgroup1 hierarchy is identified by its
  2233	 * hierarchy ID.
  2234	 * @task: The target task
  2235	 * @hierarchy_id: The ID of a cgroup1 hierarchy
  2236	 *
  2237	 * On success, the cgroup is returen. On failure, NULL is returned.
  2238	 */
  2239	__bpf_kfunc struct cgroup *
> 2240	bpf_task_get_cgroup1(struct task_struct *task, int hierarchy_id)
  2241	{
  2242		struct cgroup *cgrp = task_get_cgroup1(task, hierarchy_id);
  2243	
  2244		if (IS_ERR(cgrp))
  2245			return NULL;
  2246		return cgrp;
  2247	}
  2248	#endif /* CONFIG_CGROUPS */
  2249	

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

Re: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy

[Yafang Shao]

On Fri, Nov 3, 2023 at 4:19 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Yafang,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on bpf-next/master]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-Remove-unnecessary-list_empty/20231029-143457
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> patch link:    https://lore.kernel.org/r/20231029061438.4215-7-laoar.shao%40gmail.com
> patch subject: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy
> config: x86_64-randconfig-004-20231103 (https://download.01.org/0day-ci/archive/20231103/202311031651.A7crZEur-lkp@intel.com/config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0

__diag_ignore_all() is supported for gcc >= 8.0.0.
It seems that we should also support it for older gcc ?

> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231103/202311031651.A7crZEur-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/202311031651.A7crZEur-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
>    kernel/bpf/helpers.c:1893:19: warning: no previous prototype for 'bpf_obj_new_impl' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
>                       ^~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:1907:19: warning: no previous prototype for 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, void *meta__ign)
>                       ^~~~~~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:1941:18: warning: no previous prototype for 'bpf_obj_drop_impl' [-Wmissing-prototypes]
>     __bpf_kfunc void bpf_obj_drop_impl(void *p__alloc, void *meta__ign)
>                      ^~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:1949:18: warning: no previous prototype for 'bpf_percpu_obj_drop_impl' [-Wmissing-prototypes]
>     __bpf_kfunc void bpf_percpu_obj_drop_impl(void *p__alloc, void *meta__ign)
>                      ^~~~~~~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:1955:19: warning: no previous prototype for 'bpf_refcount_acquire_impl' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_refcount_acquire_impl(void *p__refcounted_kptr, void *meta__ign)
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2000:17: warning: no previous prototype for 'bpf_list_push_front_impl' [-Wmissing-prototypes]
>     __bpf_kfunc int bpf_list_push_front_impl(struct bpf_list_head *head,
>                     ^~~~~~~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2010:17: warning: no previous prototype for 'bpf_list_push_back_impl' [-Wmissing-prototypes]
>     __bpf_kfunc int bpf_list_push_back_impl(struct bpf_list_head *head,
>                     ^~~~~~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2043:35: warning: no previous prototype for 'bpf_list_pop_front' [-Wmissing-prototypes]
>     __bpf_kfunc struct bpf_list_node *bpf_list_pop_front(struct bpf_list_head *head)
>                                       ^~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2048:35: warning: no previous prototype for 'bpf_list_pop_back' [-Wmissing-prototypes]
>     __bpf_kfunc struct bpf_list_node *bpf_list_pop_back(struct bpf_list_head *head)
>                                       ^~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2053:33: warning: no previous prototype for 'bpf_rbtree_remove' [-Wmissing-prototypes]
>     __bpf_kfunc struct bpf_rb_node *bpf_rbtree_remove(struct bpf_rb_root *root,
>                                     ^~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2109:17: warning: no previous prototype for 'bpf_rbtree_add_impl' [-Wmissing-prototypes]
>     __bpf_kfunc int bpf_rbtree_add_impl(struct bpf_rb_root *root, struct bpf_rb_node *node,
>                     ^~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2119:33: warning: no previous prototype for 'bpf_rbtree_first' [-Wmissing-prototypes]
>     __bpf_kfunc struct bpf_rb_node *bpf_rbtree_first(struct bpf_rb_root *root)
>                                     ^~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2132:33: warning: no previous prototype for 'bpf_task_acquire' [-Wmissing-prototypes]
>     __bpf_kfunc struct task_struct *bpf_task_acquire(struct task_struct *p)
>                                     ^~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2143:18: warning: no previous prototype for 'bpf_task_release' [-Wmissing-prototypes]
>     __bpf_kfunc void bpf_task_release(struct task_struct *p)
>                      ^~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2155:28: warning: no previous prototype for 'bpf_cgroup_acquire' [-Wmissing-prototypes]
>     __bpf_kfunc struct cgroup *bpf_cgroup_acquire(struct cgroup *cgrp)
>                                ^~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2167:18: warning: no previous prototype for 'bpf_cgroup_release' [-Wmissing-prototypes]
>     __bpf_kfunc void bpf_cgroup_release(struct cgroup *cgrp)
>                      ^~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2179:28: warning: no previous prototype for 'bpf_cgroup_ancestor' [-Wmissing-prototypes]
>     __bpf_kfunc struct cgroup *bpf_cgroup_ancestor(struct cgroup *cgrp, int level)
>                                ^~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2199:28: warning: no previous prototype for 'bpf_cgroup_from_id' [-Wmissing-prototypes]
>     __bpf_kfunc struct cgroup *bpf_cgroup_from_id(u64 cgid)
>                                ^~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2219:18: warning: no previous prototype for 'bpf_task_under_cgroup' [-Wmissing-prototypes]
>     __bpf_kfunc long bpf_task_under_cgroup(struct task_struct *task,
>                      ^~~~~~~~~~~~~~~~~~~~~
> >> kernel/bpf/helpers.c:2240:1: warning: no previous prototype for 'bpf_task_get_cgroup1' [-Wmissing-prototypes]
>     bpf_task_get_cgroup1(struct task_struct *task, int hierarchy_id)
>     ^~~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2256:33: warning: no previous prototype for 'bpf_task_from_pid' [-Wmissing-prototypes]
>     __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
>                                     ^~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2297:19: warning: no previous prototype for 'bpf_dynptr_slice' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset,
>                       ^~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2381:19: warning: no previous prototype for 'bpf_dynptr_slice_rdwr' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset,
>                       ^~~~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2412:17: warning: no previous prototype for 'bpf_dynptr_adjust' [-Wmissing-prototypes]
>     __bpf_kfunc int bpf_dynptr_adjust(struct bpf_dynptr_kern *ptr, u32 start, u32 end)
>                     ^~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2430:18: warning: no previous prototype for 'bpf_dynptr_is_null' [-Wmissing-prototypes]
>     __bpf_kfunc bool bpf_dynptr_is_null(struct bpf_dynptr_kern *ptr)
>                      ^~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2435:18: warning: no previous prototype for 'bpf_dynptr_is_rdonly' [-Wmissing-prototypes]
>     __bpf_kfunc bool bpf_dynptr_is_rdonly(struct bpf_dynptr_kern *ptr)
>                      ^~~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2443:19: warning: no previous prototype for 'bpf_dynptr_size' [-Wmissing-prototypes]
>     __bpf_kfunc __u32 bpf_dynptr_size(const struct bpf_dynptr_kern *ptr)
>                       ^~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2451:17: warning: no previous prototype for 'bpf_dynptr_clone' [-Wmissing-prototypes]
>     __bpf_kfunc int bpf_dynptr_clone(struct bpf_dynptr_kern *ptr,
>                     ^~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2464:19: warning: no previous prototype for 'bpf_cast_to_kern_ctx' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_cast_to_kern_ctx(void *obj)
>                       ^~~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2469:19: warning: no previous prototype for 'bpf_rdonly_cast' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_rdonly_cast(void *obj__ign, u32 btf_id__k)
>                       ^~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2474:18: warning: no previous prototype for 'bpf_rcu_read_lock' [-Wmissing-prototypes]
>     __bpf_kfunc void bpf_rcu_read_lock(void)
>                      ^~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2479:18: warning: no previous prototype for 'bpf_rcu_read_unlock' [-Wmissing-prototypes]
>     __bpf_kfunc void bpf_rcu_read_unlock(void)
>                      ^~~~~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:2508:18: warning: no previous prototype for 'bpf_throw' [-Wmissing-prototypes]
>     __bpf_kfunc void bpf_throw(u64 cookie)
>                      ^~~~~~~~~
>    cc1: warning: unrecognized command line option '-Wno-attribute-alias'
>
>
> vim +/bpf_task_get_cgroup1 +2240 kernel/bpf/helpers.c
>
>   2229
>   2230  /**
>   2231   * bpf_task_get_cgroup1 - Acquires the associated cgroup of a task within a
>   2232   * specific cgroup1 hierarchy. The cgroup1 hierarchy is identified by its
>   2233   * hierarchy ID.
>   2234   * @task: The target task
>   2235   * @hierarchy_id: The ID of a cgroup1 hierarchy
>   2236   *
>   2237   * On success, the cgroup is returen. On failure, NULL is returned.
>   2238   */
>   2239  __bpf_kfunc struct cgroup *
> > 2240  bpf_task_get_cgroup1(struct task_struct *task, int hierarchy_id)
>   2241  {
>   2242          struct cgroup *cgrp = task_get_cgroup1(task, hierarchy_id);
>   2243
>   2244          if (IS_ERR(cgrp))
>   2245                  return NULL;
>   2246          return cgrp;
>   2247  }
>   2248  #endif /* CONFIG_CGROUPS */
>   2249
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki



-- 
Regards
Yafang

[PATCH bpf-next] compiler-gcc: Ignore -Wmissing-prototypes warning for older GCC

[Yafang Shao]

The kernel supports a minimum GCC version of 5.1.0 for building. However,
the "__diag_ignore_all" directive only suppresses the
"-Wmissing-prototypes" warning for GCC versions >= 8.0.0. As a result, when
building the kernel with older GCC versions, warnings may be triggered. The
example below illustrates the warnings reported by the kernel test robot
using GCC 7.5.0:

  compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
  All warnings (new ones prefixed by >>):

   kernel/bpf/helpers.c:1893:19: warning: no previous prototype for 'bpf_obj_new_impl' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
                      ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1907:19: warning: no previous prototype for 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, void *meta__ign)
   [...]

To address this, we should also suppress the "-Wmissing-prototypes" warning
for older GCC versions. Since "#pragma GCC diagnostic push" is supported as
of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202311031651.A7crZEur-lkp@intel.com/
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/compiler-gcc.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 7af9e34..a5cfcad 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -131,14 +131,14 @@
 #define __diag_str(s)		__diag_str1(s)
 #define __diag(s)		_Pragma(__diag_str(GCC diagnostic s))
 
-#if GCC_VERSION >= 80000
-#define __diag_GCC_8(s)		__diag(s)
+#if GCC_VERSION >= 50100
+#define __diag_GCC_5(s)		__diag(s)
 #else
-#define __diag_GCC_8(s)
+#define __diag_GCC_5(s)
 #endif
 
 #define __diag_ignore_all(option, comment) \
-	__diag_GCC(8, ignore, option)
+	__diag_GCC(5, ignore, option)
 
 /*
  * Prior to 9.1, -Wno-alloc-size-larger-than (and therefore the "alloc_size"
-- 
1.8.3.1

Re: [PATCH bpf-next] compiler-gcc: Ignore -Wmissing-prototypes warning for older GCC

[Arnd Bergmann]

On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote:
> The kernel supports a minimum GCC version of 5.1.0 for building. However,
> the "__diag_ignore_all" directive only suppresses the
> "-Wmissing-prototypes" warning for GCC versions >= 8.0.0. As a result, when
> building the kernel with older GCC versions, warnings may be triggered. The
> example below illustrates the warnings reported by the kernel test robot
> using GCC 7.5.0:
>
>   compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
>   All warnings (new ones prefixed by >>):
>
>    kernel/bpf/helpers.c:1893:19: warning: no previous prototype for 
> 'bpf_obj_new_impl' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void 
> *meta__ign)
>                       ^~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:1907:19: warning: no previous prototype for 
> 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, 
> void *meta__ign)
>    [...]
>
> To address this, we should also suppress the "-Wmissing-prototypes" warning
> for older GCC versions. Since "#pragma GCC diagnostic push" is supported as
> of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0.

Not sure why these need to be suppressed like this at all,
can't you just add the prototype somewhere?

> @@ -131,14 +131,14 @@
>  #define __diag_str(s)		__diag_str1(s)
>  #define __diag(s)		_Pragma(__diag_str(GCC diagnostic s))
> 
> -#if GCC_VERSION >= 80000
> -#define __diag_GCC_8(s)		__diag(s)
> +#if GCC_VERSION >= 50100
> +#define __diag_GCC_5(s)		__diag(s)
>  #else
> -#define __diag_GCC_8(s)
> +#define __diag_GCC_5(s)
>  #endif
> 

This breaks all uses of __diag_ignore that specify
version 8 directly. Just add the macros for each version
from 5 to 14 here.

     Arnd

Re: [PATCH bpf-next] compiler-gcc: Ignore -Wmissing-prototypes warning for older GCC

[Yafang Shao]

On Sun, Nov 5, 2023 at 4:24 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote:
> > The kernel supports a minimum GCC version of 5.1.0 for building. However,
> > the "__diag_ignore_all" directive only suppresses the
> > "-Wmissing-prototypes" warning for GCC versions >= 8.0.0. As a result, when
> > building the kernel with older GCC versions, warnings may be triggered. The
> > example below illustrates the warnings reported by the kernel test robot
> > using GCC 7.5.0:
> >
> >   compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> >   All warnings (new ones prefixed by >>):
> >
> >    kernel/bpf/helpers.c:1893:19: warning: no previous prototype for
> > 'bpf_obj_new_impl' [-Wmissing-prototypes]
> >     __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void
> > *meta__ign)
> >                       ^~~~~~~~~~~~~~~~
> >    kernel/bpf/helpers.c:1907:19: warning: no previous prototype for
> > 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes]
> >     __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k,
> > void *meta__ign)
> >    [...]
> >
> > To address this, we should also suppress the "-Wmissing-prototypes" warning
> > for older GCC versions. Since "#pragma GCC diagnostic push" is supported as
> > of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0.
>
> Not sure why these need to be suppressed like this at all,
> can't you just add the prototype somewhere?

BPF kfuncs are intended for use within BPF programs, and they should
not be called from other parts of the kernel. Consequently, it is not
appropriate to include their prototypes in a kernel header file.

>
> > @@ -131,14 +131,14 @@
> >  #define __diag_str(s)                __diag_str1(s)
> >  #define __diag(s)            _Pragma(__diag_str(GCC diagnostic s))
> >
> > -#if GCC_VERSION >= 80000
> > -#define __diag_GCC_8(s)              __diag(s)
> > +#if GCC_VERSION >= 50100
> > +#define __diag_GCC_5(s)              __diag(s)
> >  #else
> > -#define __diag_GCC_8(s)
> > +#define __diag_GCC_5(s)
> >  #endif
> >
>
> This breaks all uses of __diag_ignore that specify
> version 8 directly. Just add the macros for each version
> from 5 to 14 here.

It seems that __diag_GCC_8() or __diag_GCC() are not directly used
anywhere in the kernel, right?
Therefore it won't break anything if we just replace __diag_GCC_8()
with __diag_GCC_5().
It may be cumbersome to add the macrocs for every GCC version if they
aren't actively used.

--
Regards
Yafang

Re: [PATCH bpf-next] compiler-gcc: Ignore -Wmissing-prototypes warning for older GCC

[Arnd Bergmann]

On Sun, Nov 5, 2023, at 12:54, Yafang Shao wrote:
> On Sun, Nov 5, 2023 at 4:24 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote:
>> > To address this, we should also suppress the "-Wmissing-prototypes" warning
>> > for older GCC versions. Since "#pragma GCC diagnostic push" is supported as
>> > of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0.
>>
>> Not sure why these need to be suppressed like this at all,
>> can't you just add the prototype somewhere?
>
> BPF kfuncs are intended for use within BPF programs, and they should
> not be called from other parts of the kernel. Consequently, it is not
> appropriate to include their prototypes in a kernel header file.

How does the caller in the BPF program get the prototype?

>> > @@ -131,14 +131,14 @@
>> >  #define __diag_str(s)                __diag_str1(s)
>> >  #define __diag(s)            _Pragma(__diag_str(GCC diagnostic s))
>> >
>> > -#if GCC_VERSION >= 80000
>> > -#define __diag_GCC_8(s)              __diag(s)
>> > +#if GCC_VERSION >= 50100
>> > +#define __diag_GCC_5(s)              __diag(s)
>> >  #else
>> > -#define __diag_GCC_8(s)
>> > +#define __diag_GCC_5(s)
>> >  #endif
>> >
>>
>> This breaks all uses of __diag_ignore that specify
>> version 8 directly. Just add the macros for each version
>> from 5 to 14 here.
>
> It seems that __diag_GCC_8() or __diag_GCC() are not directly used
> anywhere in the kernel, right?

I see three instances:

drivers/net/ethernet/renesas/sh_eth.c:__diag_ignore(GCC, 8, "-Woverride-init",
include/linux/compat.h:     __diag_ignore(GCC, 8, "-Wattribute-alias",                              include/linux/syscalls.h:   __diag_ignore(GCC, 8, "-Wattribute-alias",                      

The override-init one should probably use version 5 as well,
but I think the -Wattribute-alias ones require GCC 8 and otherwise
cause a warning about an unknown warning option.

__diag_ignore_all() would also be wrong for the override-init
because the option has a different name in clang
(-Winitializer-overrides).

> Therefore it won't break anything if we just replace __diag_GCC_8()
> with __diag_GCC_5().
> It may be cumbersome to add the macrocs for every GCC version if they
> aren't actively used.

For the _all variant, I would prefer to completely remove
the version logic and just use __diag() directly. I think the
entire point of this is that it is used on all supported
versions.

      Arnd

Re: [PATCH bpf-next] compiler-gcc: Ignore -Wmissing-prototypes warning for older GCC

[Yafang Shao]

On Sun, Nov 5, 2023 at 9:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sun, Nov 5, 2023, at 12:54, Yafang Shao wrote:
> > On Sun, Nov 5, 2023 at 4:24 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >> On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote:
> >> > To address this, we should also suppress the "-Wmissing-prototypes" warning
> >> > for older GCC versions. Since "#pragma GCC diagnostic push" is supported as
> >> > of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0.
> >>
> >> Not sure why these need to be suppressed like this at all,
> >> can't you just add the prototype somewhere?
> >
> > BPF kfuncs are intended for use within BPF programs, and they should
> > not be called from other parts of the kernel. Consequently, it is not
> > appropriate to include their prototypes in a kernel header file.
>
> How does the caller in the BPF program get the prototype?

BPF programs will get the prototype directly from BTF, for example,
see also the prototypes declared using "__ksym"  in
tools/testing/selftests/bpf/progs/ for examples.

>
> >> > @@ -131,14 +131,14 @@
> >> >  #define __diag_str(s)                __diag_str1(s)
> >> >  #define __diag(s)            _Pragma(__diag_str(GCC diagnostic s))
> >> >
> >> > -#if GCC_VERSION >= 80000
> >> > -#define __diag_GCC_8(s)              __diag(s)
> >> > +#if GCC_VERSION >= 50100
> >> > +#define __diag_GCC_5(s)              __diag(s)
> >> >  #else
> >> > -#define __diag_GCC_8(s)
> >> > +#define __diag_GCC_5(s)
> >> >  #endif
> >> >
> >>
> >> This breaks all uses of __diag_ignore that specify
> >> version 8 directly. Just add the macros for each version
> >> from 5 to 14 here.
> >
> > It seems that __diag_GCC_8() or __diag_GCC() are not directly used
> > anywhere in the kernel, right?
>
> I see three instances:
>
> drivers/net/ethernet/renesas/sh_eth.c:__diag_ignore(GCC, 8, "-Woverride-init",
> include/linux/compat.h:     __diag_ignore(GCC, 8, "-Wattribute-alias",                              include/linux/syscalls.h:   __diag_ignore(GCC, 8, "-Wattribute-alias",

Thanks for pointing them out.

>
> The override-init one should probably use version 5 as well,
> but I think the -Wattribute-alias ones require GCC 8 and otherwise
> cause a warning about an unknown warning option.

Right. -Wattribute-alias requires GCC 8.

>
> __diag_ignore_all() would also be wrong for the override-init
> because the option has a different name in clang
> (-Winitializer-overrides).
>
> > Therefore it won't break anything if we just replace __diag_GCC_8()
> > with __diag_GCC_5().
> > It may be cumbersome to add the macrocs for every GCC version if they
> > aren't actively used.
>
> For the _all variant, I would prefer to completely remove
> the version logic and just use __diag() directly. I think the
> entire point of this is that it is used on all supported
> versions.

Good suggestion.
will do it.

-- 
Regards
Yafang

[PATCH v2 bpf-next] compiler-gcc: Suppress -Wmissing-prototypes warning for all supported GCC

[Yafang Shao]

The kernel supports a minimum GCC version of 5.1.0 for building. However,
the "__diag_ignore_all" directive only suppresses the
"-Wmissing-prototypes" warning for GCC versions >= 8.0.0. As a result, when
building the kernel with older GCC versions, warnings may be triggered. The
example below illustrates the warnings reported by the kernel test robot
using GCC 7.5.0:

  compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
  All warnings (new ones prefixed by >>):

   kernel/bpf/helpers.c:1893:19: warning: no previous prototype for 'bpf_obj_new_impl' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
                      ^~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1907:19: warning: no previous prototype for 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes]
    __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, void *meta__ign)
   [...]

To address this, we should also suppress the "-Wmissing-prototypes" warning
for older GCC versions. "#pragma GCC diagnostic push" is supported as
of GCC 4.6, and both "-Wmissing-prototypes" and "-Wmissing-declarations"
are supported for all the GCC versions that we currently support.
Therefore, it is reasonable to suppress these warnings for all supported
GCC versions.

With this adjustment, it's important to note that after implementing
"__diag_ignore_all", it will effectively suppress warnings for all the
supported GCC versions.

In the future, if you wish to suppress warnings that are only supported on
higher GCC versions, it is advisable to explicitly use "__diag_ignore" to
specify the GCC version you are targeting.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202311031651.A7crZEur-lkp@intel.com/
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/compiler-gcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 7af9e34..80918bd 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -138,7 +138,7 @@
 #endif
 
 #define __diag_ignore_all(option, comment) \
-	__diag_GCC(8, ignore, option)
+	__diag(__diag_GCC_ignore option)
 
 /*
  * Prior to 9.1, -Wno-alloc-size-larger-than (and therefore the "alloc_size"
-- 
1.8.3.1

Re: [PATCH v2 bpf-next] compiler-gcc: Suppress -Wmissing-prototypes warning for all supported GCC

[Alexei Starovoitov]

On Sun, Nov 5, 2023 at 7:18 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> The kernel supports a minimum GCC version of 5.1.0 for building. However,
> the "__diag_ignore_all" directive only suppresses the
> "-Wmissing-prototypes" warning for GCC versions >= 8.0.0. As a result, when
> building the kernel with older GCC versions, warnings may be triggered. The
> example below illustrates the warnings reported by the kernel test robot
> using GCC 7.5.0:
>
>   compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
>   All warnings (new ones prefixed by >>):
>
>    kernel/bpf/helpers.c:1893:19: warning: no previous prototype for 'bpf_obj_new_impl' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
>                       ^~~~~~~~~~~~~~~~
>    kernel/bpf/helpers.c:1907:19: warning: no previous prototype for 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes]
>     __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, void *meta__ign)
>    [...]
>
> To address this, we should also suppress the "-Wmissing-prototypes" warning
> for older GCC versions. "#pragma GCC diagnostic push" is supported as
> of GCC 4.6, and both "-Wmissing-prototypes" and "-Wmissing-declarations"
> are supported for all the GCC versions that we currently support.
> Therefore, it is reasonable to suppress these warnings for all supported
> GCC versions.
>
> With this adjustment, it's important to note that after implementing
> "__diag_ignore_all", it will effectively suppress warnings for all the
> supported GCC versions.
>
> In the future, if you wish to suppress warnings that are only supported on
> higher GCC versions, it is advisable to explicitly use "__diag_ignore" to
> specify the GCC version you are targeting.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202311031651.A7crZEur-lkp@intel.com/
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/linux/compiler-gcc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 7af9e34..80918bd 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -138,7 +138,7 @@
>  #endif
>
>  #define __diag_ignore_all(option, comment) \
> -       __diag_GCC(8, ignore, option)
> +       __diag(__diag_GCC_ignore option)

Arnd,
does this look good to you?

If so, pls ack.

Re: [PATCH v2 bpf-next] compiler-gcc: Suppress -Wmissing-prototypes warning for all supported GCC

[Arnd Bergmann]

On Thu, Nov 9, 2023, at 19:23, Alexei Starovoitov wrote:
> On Sun, Nov 5, 2023 at 7:18 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>> In the future, if you wish to suppress warnings that are only supported on
>> higher GCC versions, it is advisable to explicitly use "__diag_ignore" to
>> specify the GCC version you are targeting.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202311031651.A7crZEur-lkp@intel.com/
>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
>> Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> ---
>>  include/linux/compiler-gcc.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>> index 7af9e34..80918bd 100644
>> --- a/include/linux/compiler-gcc.h
>> +++ b/include/linux/compiler-gcc.h
>> @@ -138,7 +138,7 @@
>>  #endif
>>
>>  #define __diag_ignore_all(option, comment) \
>> -       __diag_GCC(8, ignore, option)
>> +       __diag(__diag_GCC_ignore option)
>
> Arnd,
> does this look good to you?

Yes, this is good. We could do the same thing for 
clang already, but it doesn't make a huge difference.

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH v2 bpf-next] compiler-gcc: Suppress -Wmissing-prototypes warning for all supported GCC

[Yafang Shao]

On Fri, Nov 10, 2023 at 2:35 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Nov 9, 2023, at 19:23, Alexei Starovoitov wrote:
> > On Sun, Nov 5, 2023 at 7:18 PM Yafang Shao <laoar.shao@gmail.com> wrote:
> >> In the future, if you wish to suppress warnings that are only supported on
> >> higher GCC versions, it is advisable to explicitly use "__diag_ignore" to
> >> specify the GCC version you are targeting.
> >>
> >> Reported-by: kernel test robot <lkp@intel.com>
> >> Closes: https://lore.kernel.org/oe-kbuild-all/202311031651.A7crZEur-lkp@intel.com/
> >> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> >> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> >> Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >>  include/linux/compiler-gcc.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> >> index 7af9e34..80918bd 100644
> >> --- a/include/linux/compiler-gcc.h
> >> +++ b/include/linux/compiler-gcc.h
> >> @@ -138,7 +138,7 @@
> >>  #endif
> >>
> >>  #define __diag_ignore_all(option, comment) \
> >> -       __diag_GCC(8, ignore, option)
> >> +       __diag(__diag_GCC_ignore option)
> >
> > Arnd,
> > does this look good to you?
>
> Yes, this is good. We could do the same thing for
> clang already, but it doesn't make a huge difference.

The Minimum Clang version is 11.0.0, so I think we don't have to
change compiler-clang.h.

>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Thanks for your review.

-- 
Regards
Yafang

Re: [PATCH v2 bpf-next] compiler-gcc: Suppress -Wmissing-prototypes warning for all supported GCC

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Mon,  6 Nov 2023 03:18:02 +0000 you wrote:
> The kernel supports a minimum GCC version of 5.1.0 for building. However,
> the "__diag_ignore_all" directive only suppresses the
> "-Wmissing-prototypes" warning for GCC versions >= 8.0.0. As a result, when
> building the kernel with older GCC versions, warnings may be triggered. The
> example below illustrates the warnings reported by the kernel test robot
> using GCC 7.5.0:
> 
> [...]

Here is the summary with links:
  - [v2,bpf-next] compiler-gcc: Suppress -Wmissing-prototypes warning for all supported GCC
    https://git.kernel.org/bpf/bpf-next/c/689b097a06ba

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v3 bpf-next 06/11] bpf: Add a new kfunc for cgroup1 hierarchy

[Tejun Heo]

On Sun, Oct 29, 2023 at 06:14:33AM +0000, Yafang Shao wrote:
> A new kfunc is added to acquire cgroup1 of a task:
> 
> - bpf_task_get_cgroup1
>   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.
> 
> Suggested-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

[PATCH v3 bpf-next 07/11] 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 5b1da2a..10b5f42 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();
-- 
1.8.3.1

[PATCH v3 bpf-next 08/11] 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 +-
 tools/testing/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 10b5f42..f18649a 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 5c2cb9c..92fc41d 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.h
+++ b/tools/testing/selftests/bpf/cgroup_helpers.h
@@ -29,7 +29,7 @@ int write_cgroup_file_parent(const char *relative_path, const char *file,
 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 9026b42..addf720 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);
-- 
1.8.3.1

[PATCH v3 bpf-next 09/11] 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 f18649a..63bfa72 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 92fc41d..e71da4e 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.h
+++ b/tools/testing/selftests/bpf/cgroup_helpers.h
@@ -31,6 +31,7 @@ int write_cgroup_file_parent(const char *relative_path, const char *file,
 /* 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);
-- 
1.8.3.1

[PATCH v3 bpf-next 10/11] 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 63bfa72..d75bb87 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 e71da4e..a80c417 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.h
+++ b/tools/testing/selftests/bpf/cgroup_helpers.h
@@ -20,6 +20,7 @@ int write_cgroup_file_parent(const char *relative_path, const char *file,
 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);
-- 
1.8.3.1

Re: [PATCH v3 bpf-next 10/11] selftests/bpf: Add a new cgroup helper get_cgroup_hierarchy_id()

[Tejun Heo]

Hello,

On Sun, Oct 29, 2023 at 06:14:37AM +0000, Yafang Shao wrote:
> +int get_cgroup1_hierarchy_id(const char *cgrp_name)

Maybe use subsys_name or controller? cgroup name usually means the cgroup
directory name.

Thanks.

-- 
tejun

Re: [PATCH v3 bpf-next 10/11] selftests/bpf: Add a new cgroup helper get_cgroup_hierarchy_id()

[Yafang Shao]

On Fri, Nov 10, 2023 at 5:32 AM Tejun Heo <tj@kernel.org> wrote:
>
> Hello,
>
> On Sun, Oct 29, 2023 at 06:14:37AM +0000, Yafang Shao wrote:
> > +int get_cgroup1_hierarchy_id(const char *cgrp_name)
>
> Maybe use subsys_name or controller? cgroup name usually means the cgroup
> directory name.
>

will use subsys_name instead. Thanks for your suggestion.

-- 
Regards
Yafang

[PATCH v3 bpf-next 11/11] 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

Besides, I also did some stress test similar to the patch #2 in this
series, as follows (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 run this selftest concurrently,
  while true; do ./test_progs --name=cgroup1_hierarchy; done

They can ran successfully without any RCU warnings in dmesg.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 .../selftests/bpf/prog_tests/cgroup1_hierarchy.c   | 159 +++++++++++++++++++++
 .../selftests/bpf/progs/test_cgroup1_hierarchy.c   |  72 ++++++++++
 2 files changed, 231 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 0000000..4aafbc9
--- /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 0000000..979ff4e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c
@@ -0,0 +1,72 @@
+// 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(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(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";
-- 
1.8.3.1

Re: [PATCH v3 bpf-next 00/11] bpf, cgroup: Add BPF support for cgroup1 hierarchy

[Tejun Heo]

Hello,

On Sun, Oct 29, 2023 at 06:14:27AM +0000, Yafang Shao wrote:
> - [bpf_]task_get_cgroup1
>   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.

Sans minor nits, the whole series looks good to me. I can take the cgroup
prep patches through cgroup tree but it's also fine to route them through
the bpf tree with the rest of the series. Please let me know how folks want
to route the series once the minor issues are addressed.

Thanks.

-- 
tejun

Re: [PATCH v3 bpf-next 00/11] bpf, cgroup: Add BPF support for cgroup1 hierarchy

[Alexei Starovoitov]

On Thu, Nov 9, 2023 at 1:36 PM Tejun Heo <tj@kernel.org> wrote:
>
> Hello,
>
> On Sun, Oct 29, 2023 at 06:14:27AM +0000, Yafang Shao wrote:
> > - [bpf_]task_get_cgroup1
> >   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.
>
> Sans minor nits, the whole series looks good to me. I can take the cgroup
> prep patches through cgroup tree but it's also fine to route them through
> the bpf tree with the rest of the series. Please let me know how folks want
> to route the series once the minor issues are addressed.

Do you think there could be conflicts between cgroup and bpf trees ?
If so, maybe you can push the cgroup patches into the stable branch and pull it
into the main cgroup branch and we'll pull the same branch into bpf-next.
This way all shas will remain and no conflicts during the merge window.

Re: [PATCH v3 bpf-next 00/11] bpf, cgroup: Add BPF support for cgroup1 hierarchy

[Tejun Heo]

Hello,

Applied 1-5 to cgroup/for-6.8-bpf. The last patch is updated to use
irqsave/restore. Will post the updated version as a reply to the original
patch.

  git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-6.8-bpf

Alexei, please feel free to pull from the branch. It's stable and will also
be included as a part of cgroup/for-6.8.

Thanks.

-- 
tejun

Re: [PATCH v3 bpf-next 00/11] bpf, cgroup: Add BPF support for cgroup1 hierarchy

[Alexei Starovoitov]

On Thu, Nov 9, 2023 at 3:28 PM Tejun Heo <tj@kernel.org> wrote:
>
> Hello,
>
> Applied 1-5 to cgroup/for-6.8-bpf. The last patch is updated to use
> irqsave/restore. Will post the updated version as a reply to the original
> patch.
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-6.8-bpf
>
> Alexei, please feel free to pull from the branch. It's stable and will also
> be included as a part of cgroup/for-6.8.

Perfect. Thanks.
Will probably pull it either tomorrow or on Monday/Tuesday.

Re: [PATCH v3 bpf-next 00/11] bpf, cgroup: Add BPF support for cgroup1 hierarchy

[Yafang Shao]

On Fri, Nov 10, 2023 at 7:35 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Nov 9, 2023 at 3:28 PM Tejun Heo <tj@kernel.org> wrote:
> >
> > Hello,
> >
> > Applied 1-5 to cgroup/for-6.8-bpf. The last patch is updated to use
> > irqsave/restore. Will post the updated version as a reply to the original
> > patch.
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-6.8-bpf
> >
> > Alexei, please feel free to pull from the branch. It's stable and will also
> > be included as a part of cgroup/for-6.8.
>
> Perfect. Thanks.
> Will probably pull it either tomorrow or on Monday/Tuesday.

will send a new version for the other parts after you pull it.

-- 
Regards
Yafang

Re: [PATCH v3 bpf-next 00/11] bpf, cgroup: Add BPF support for cgroup1 hierarchy

[Alexei Starovoitov]

On Thu, Nov 9, 2023 at 10:05 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Fri, Nov 10, 2023 at 7:35 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Thu, Nov 9, 2023 at 3:28 PM Tejun Heo <tj@kernel.org> wrote:
> > >
> > > Hello,
> > >
> > > Applied 1-5 to cgroup/for-6.8-bpf. The last patch is updated to use
> > > irqsave/restore. Will post the updated version as a reply to the original
> > > patch.
> > >
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-6.8-bpf
> > >
> > > Alexei, please feel free to pull from the branch. It's stable and will also
> > > be included as a part of cgroup/for-6.8.
> >
> > Perfect. Thanks.
> > Will probably pull it either tomorrow or on Monday/Tuesday.
>
> will send a new version for the other parts after you pull it.

Pulled into bpf-next.