Re: [PATCH v3 3/4] block: add BPF kfuncs to read blkcg io.stat

[email protected]
Newsgroups org.kernel.vger.cgroups,org.kernel.vger.bpf,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest
Message-ID <8314f1b2a808075d369a73829c884b05258dafbc581ba56e981ad5557bdbfc47@mail.kernel.org>
> diff --git a/kernel/cgroup/bpf_cgroup.c b/kernel/cgroup/bpf_cgroup.c
> index cd28c838dc7b..e253633e8278 100644
> --- a/kernel/cgroup/bpf_cgroup.c
> +++ b/kernel/cgroup/bpf_cgroup.c
> @@ -8,12 +8,50 @@
>  #include <linux/bpf.h>
>  #include <linux/btf_ids.h>
>  #include <linux/cgroup.h>
> +#include <linux/rcupdate.h>
>
> +#ifdef CONFIG_CGROUP_SCHED
>  #include "../sched/sched.h"
> +#endif
>
> -#ifdef CONFIG_CGROUP_SCHED
>  __bpf_kfunc_start_defs();
>
> +/**
> + * bpf_cgroup_css - Get a reference to one controller's css
> + * @cgrp: cgroup to look in
> + * @ssid: controller ID
> + *
> + * The returned css must be released with bpf_css_release().
> + *
> + * Return: The referenced css, or NULL.
> + */
> +__bpf_kfunc struct cgroup_subsys_state *
> +bpf_cgroup_css(struct cgroup *cgrp, int ssid)
> +{
> +	struct cgroup_subsys_state *css;
> +
> +	if (unlikely(ssid < 0 || ssid >= CGROUP_SUBSYS_COUNT))
> +		return NULL;
> +
> +	rcu_read_lock();
> +	css = rcu_dereference(cgrp->subsys[ssid]);
> +	if (css && !css_tryget(css))
> +		css = NULL;
> +	rcu_read_unlock();
> +
> +	return css;
> +}
> +
> +/**
> + * bpf_css_release - Release a css reference
> + * @css: css to release
> + */
> +__bpf_kfunc void bpf_css_release(struct cgroup_subsys_state *css)
> +{
> +	css_put(css);
> +}
> +
> +#ifdef CONFIG_CGROUP_SCHED

[ ... ]

> @@ -30,29 +68,33 @@ bpf_css_to_task_group(struct cgroup_subsys_state *css)
>
>  	return container_of(css, struct task_group, css);
>  }
> +#endif /* CONFIG_CGROUP_SCHED */
>
>  __bpf_kfunc_end_defs();
>
> -BTF_KFUNCS_START(bpf_cpu_cgroup_kfunc_ids)
> +BTF_KFUNCS_START(bpf_cgroup_kfunc_ids)
> +BTF_ID_FLAGS(func, bpf_cgroup_css, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
> +BTF_ID_FLAGS(func, bpf_css_release, KF_RELEASE)
> +#ifdef CONFIG_CGROUP_SCHED
>  BTF_ID_FLAGS(func, bpf_css_to_task_group,
>  	     KF_RCU | KF_RCU_PROTECTED | KF_RET_NULL)
> -BTF_KFUNCS_END(bpf_cpu_cgroup_kfunc_ids)
> +#endif
> +BTF_KFUNCS_END(bpf_cgroup_kfunc_ids)
>
> -static const struct btf_kfunc_id_set bpf_cpu_cgroup_kfunc_set = {
> +static const struct btf_kfunc_id_set bpf_cgroup_kfunc_set = {
>  	.owner		= THIS_MODULE,
> -	.set		= &bpf_cpu_cgroup_kfunc_ids,
> +	.set		= &bpf_cgroup_kfunc_ids,
>  };
>
> -static int __init bpf_cpu_cgroup_kfunc_init(void)
> +static int __init bpf_cgroup_kfunc_init(void)
>  {
>  	int err;
>
>  	err = register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC,
> -					&bpf_cpu_cgroup_kfunc_set);
> +					&bpf_cgroup_kfunc_set);
>  	if (err)
> -		pr_warn("error while registering cpu cgroup kfuncs: %d\n", err);
> +		pr_warn("error while registering cgroup kfuncs: %d\n", err);
>
>  	return err;
>  }
> -late_initcall(bpf_cpu_cgroup_kfunc_init);
> -#endif /* CONFIG_CGROUP_SCHED */
> +late_initcall(bpf_cgroup_kfunc_init);

This isn't a bug, but the patch adds two controller-agnostic kfuncs
(bpf_cgroup_css and bpf_css_release) in kernel/cgroup/bpf_cgroup.c, along
with a three-way rename (bpf_cpu_cgroup_kfunc_ids to bpf_cgroup_kfunc_ids)
and an ifdef restructure of the whole file.

The immediately preceding commit in this series (382c3bd4 "cgroup: add BPF
kfuncs to read a cpu cgroup's stats") already touched this same file under
a "cgroup:" prefix.

Would it be worth splitting the generic css acquire/release and the rename
into a separate "cgroup:" patch, so this one stays scoped to the block
changes?


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

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32419919743
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.