[PATCH v2 0/2] Support cgroup1 in get from fd/file interfaces

[PATCH v2 0/2] Support cgroup1 in get from fd/file interfaces

[Yosry Ahmed]

commit f3a2aebdd6fb ("cgroup: enable cgroup_get_from_file() on cgroup1")
enabled using cgroup_get_from_file() and cgroup_get_from_fd() on
cgroup1, to enable bpf cgroup_iter to attach to cgroup1.

Apparently, other callers depended on these functions only supporting
cgroup2, so f3a2aebdd6 was reverted. Instead, add new separate interfaces
that support both cgroup1 and cgroup2 and use them in bpf cgroup_iter.

Yosry Ahmed (2):
  cgroup: add cgroup_v1v2_get_from_[fd/file]()
  bpf: cgroup_iter: support cgroup1 using cgroup fd

 include/linux/cgroup.h   |  1 +
 kernel/bpf/cgroup_iter.c |  2 +-
 kernel/cgroup/cgroup.c   | 50 +++++++++++++++++++++++++++++++++++-----
 3 files changed, 46 insertions(+), 7 deletions(-)

-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v2 1/2] cgroup: add cgroup_v1v2_get_from_[fd/file]()

[Yosry Ahmed]

Add cgroup_v1v2_get_from_fd() and cgroup_v1v2_get_from_file() that
support both cgroup1 and cgroup2.

Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
---
 include/linux/cgroup.h |  1 +
 kernel/cgroup/cgroup.c | 50 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 398f0bce7c21..a88de5bdeaa9 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -106,6 +106,7 @@ struct cgroup_subsys_state *css_tryget_online_from_dir(struct dentry *dentry,
 
 struct cgroup *cgroup_get_from_path(const char *path);
 struct cgroup *cgroup_get_from_fd(int fd);
+struct cgroup *cgroup_v1v2_get_from_fd(int fd);
 
 int cgroup_attach_task_all(struct task_struct *from, struct task_struct *);
 int cgroup_transfer_tasks(struct cgroup *to, struct cgroup *from);
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 72e97422e9d9..be167e15ef1a 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6208,16 +6208,36 @@ void cgroup_fork(struct task_struct *child)
 	INIT_LIST_HEAD(&child->cg_list);
 }
 
-static struct cgroup *cgroup_get_from_file(struct file *f)
+/**
+ * cgroup_v1v2_get_from_file - get a cgroup pointer from a file pointer
+ * @f: file corresponding to cgroup_dir
+ *
+ * Find the cgroup from a file pointer associated with a cgroup directory.
+ * Returns a pointer to the cgroup on success. ERR_PTR is returned if the
+ * cgroup cannot be found.
+ */
+static struct cgroup *cgroup_v1v2_get_from_file(struct file *f)
 {
 	struct cgroup_subsys_state *css;
-	struct cgroup *cgrp;
 
 	css = css_tryget_online_from_dir(f->f_path.dentry, NULL);
 	if (IS_ERR(css))
 		return ERR_CAST(css);
 
-	cgrp = css->cgroup;
+	return css->cgroup;
+}
+
+/**
+ * cgroup_get_from_file - same as cgroup_v1v2_get_from_file, but only supports
+ * cgroup2.
+ */
+static struct cgroup *cgroup_get_from_file(struct file *f)
+{
+	struct cgroup *cgrp = cgroup_v1v2_get_from_file(f);
+
+	if (IS_ERR(cgrp))
+		return ERR_CAST(cgrp);
+
 	if (!cgroup_on_dfl(cgrp)) {
 		cgroup_put(cgrp);
 		return ERR_PTR(-EBADF);
@@ -6720,14 +6740,14 @@ EXPORT_SYMBOL_GPL(cgroup_get_from_path);
 
 /**
  * cgroup_get_from_fd - get a cgroup pointer from a fd
- * @fd: fd obtained by open(cgroup2_dir)
+ * @fd: fd obtained by open(cgroup_dir)
  *
  * Find the cgroup from a fd which should be obtained
  * by opening a cgroup directory.  Returns a pointer to the
  * cgroup on success. ERR_PTR is returned if the cgroup
  * cannot be found.
  */
-struct cgroup *cgroup_get_from_fd(int fd)
+struct cgroup *cgroup_v1v2_get_from_fd(int fd)
 {
 	struct cgroup *cgrp;
 	struct file *f;
@@ -6736,10 +6756,28 @@ struct cgroup *cgroup_get_from_fd(int fd)
 	if (!f)
 		return ERR_PTR(-EBADF);
 
-	cgrp = cgroup_get_from_file(f);
+	cgrp = cgroup_v1v2_get_from_file(f);
 	fput(f);
 	return cgrp;
 }
+
+/**
+ * cgroup_get_from_fd - same as cgroup_v1v2_get_from_fd, but only supports
+ * cgroup2.
+ */
+struct cgroup *cgroup_get_from_fd(int fd)
+{
+	struct cgroup *cgrp = cgroup_v1v2_get_from_fd(fd);
+
+	if (IS_ERR(cgrp))
+		return ERR_CAST(cgrp);
+
+	if (!cgroup_on_dfl(cgrp)) {
+		cgroup_put(cgrp);
+		return ERR_PTR(-EBADF);
+	}
+	return cgrp;
+}
 EXPORT_SYMBOL_GPL(cgroup_get_from_fd);
 
 static u64 power_of_ten(int power)
-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v2 2/2] bpf: cgroup_iter: support cgroup1 using cgroup fd

[Yosry Ahmed]

Use cgroup_v1v2_get_from_fd() in cgroup_iter to support attaching to
both cgroup v1 and v2 using fds.

Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
---
 kernel/bpf/cgroup_iter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
index 0d200a993489..9fcf09f2ef00 100644
--- a/kernel/bpf/cgroup_iter.c
+++ b/kernel/bpf/cgroup_iter.c
@@ -196,7 +196,7 @@ static int bpf_iter_attach_cgroup(struct bpf_prog *prog,
 		return -EINVAL;
 
 	if (fd)
-		cgrp = cgroup_get_from_fd(fd);
+		cgrp = cgroup_v1v2_get_from_fd(fd);
 	else if (id)
 		cgrp = cgroup_get_from_id(id);
 	else /* walk the entire hierarchy by default. */
-- 
2.38.0.rc1.362.ged0d419d3c-goog

Re: [PATCH v2 2/2] bpf: cgroup_iter: support cgroup1 using cgroup fd

[Martin KaFai Lau]

On 10/10/22 5:33 PM, Yosry Ahmed wrote:
> Use cgroup_v1v2_get_from_fd() in cgroup_iter to support attaching to
> both cgroup v1 and v2 using fds.
> 
> Signed-off-by: Yosry Ahmed <yosryahmed-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
>   kernel/bpf/cgroup_iter.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
> index 0d200a993489..9fcf09f2ef00 100644
> --- a/kernel/bpf/cgroup_iter.c
> +++ b/kernel/bpf/cgroup_iter.c
> @@ -196,7 +196,7 @@ static int bpf_iter_attach_cgroup(struct bpf_prog *prog,
>   		return -EINVAL;
>   
>   	if (fd)
> -		cgrp = cgroup_get_from_fd(fd);
> +		cgrp = cgroup_v1v2_get_from_fd(fd);

Acked-by: Martin KaFai Lau <martin.lau-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Tejun, patch 1 should depend on a recent revert that is not in the bpf tree yet. 
  Do you want to take this set to the cgroup tree?

>   	else if (id)
>   		cgrp = cgroup_get_from_id(id);
>   	else /* walk the entire hierarchy by default. */

Re: [PATCH v2 2/2] bpf: cgroup_iter: support cgroup1 using cgroup fd

[Tejun Heo]

On Tue, Oct 11, 2022 at 09:46:27AM -0700, Martin KaFai Lau wrote:
> On 10/10/22 5:33 PM, Yosry Ahmed wrote:
> > Use cgroup_v1v2_get_from_fd() in cgroup_iter to support attaching to
> > both cgroup v1 and v2 using fds.
> > 
> > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> > ---
> >   kernel/bpf/cgroup_iter.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
> > index 0d200a993489..9fcf09f2ef00 100644
> > --- a/kernel/bpf/cgroup_iter.c
> > +++ b/kernel/bpf/cgroup_iter.c
> > @@ -196,7 +196,7 @@ static int bpf_iter_attach_cgroup(struct bpf_prog *prog,
> >   		return -EINVAL;
> >   	if (fd)
> > -		cgrp = cgroup_get_from_fd(fd);
> > +		cgrp = cgroup_v1v2_get_from_fd(fd);
> 
> Acked-by: Martin KaFai Lau <martin.lau@kernel.org>
> 
> Tejun, patch 1 should depend on a recent revert that is not in the bpf tree
> yet.  Do you want to take this set to the cgroup tree?

Yeah, will do.

Thanks.

-- 
tejun

Re: [PATCH v2 0/2] Support cgroup1 in get from fd/file interfaces

[Tejun Heo]

On Tue, Oct 11, 2022 at 12:33:57AM +0000, Yosry Ahmed wrote:
> commit f3a2aebdd6fb ("cgroup: enable cgroup_get_from_file() on cgroup1")
> enabled using cgroup_get_from_file() and cgroup_get_from_fd() on
> cgroup1, to enable bpf cgroup_iter to attach to cgroup1.
> 
> Apparently, other callers depended on these functions only supporting
> cgroup2, so f3a2aebdd6 was reverted. Instead, add new separate interfaces
> that support both cgroup1 and cgroup2 and use them in bpf cgroup_iter.

Applied to cgroup/for-6.1-fixes.

Thanks.

-- 
tejun