+cc cgroup people, please do include them on this stuff.
BTW I see there is a BPF [STORAGE & CGROUPS] section in MAINTAINERS and
kernel/bpf/cgroup.c etc. anything useful there for us?
On Tue, Aug 26, 2025 at 03:19:40PM +0800, Yafang Shao wrote:
> We will utilize this new kfunc bpf_mm_get_mem_cgroup() to retrieve the
> associated mem_cgroup from the given @mm. The obtained mem_cgroup must
> be released by calling bpf_put_mem_cgroup() as a paired operation.
What locking guarantees do we have that this is all fine?
>
> Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> ---
> mm/bpf_thp.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
Also not to be nitty (but I'm going to be anyway :P) but I'm not in love with
the filename here.
So now we have
- khugepaged.c
- huge_memory.c
- bpf_thp.c
Let's maybe call it huge_memory_bpf.c for consistency? And obv as mentioned
before, add it to the MAINTAINERS in the THP section plz.
> 1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/mm/bpf_thp.c b/mm/bpf_thp.c
> index fbff3b1bb988..b757e8f425fd 100644
> --- a/mm/bpf_thp.c
> +++ b/mm/bpf_thp.c
> @@ -175,10 +175,59 @@ static struct bpf_struct_ops bpf_bpf_thp_ops = {
> .name = "bpf_thp_ops",
> };
>
> +__bpf_kfunc_start_defs();
> +
> +/**
> + * bpf_mm_get_mem_cgroup - Get the memory cgroup associated with a mm_struct.
> + * @mm: The mm_struct to query
> + *
> + * The obtained mem_cgroup must be released by calling bpf_put_mem_cgroup().
> + *
> + * Return: The associated mem_cgroup on success, or NULL on failure. Note that
> + * this function depends on CONFIG_MEMCG being enabled - it will always return
> + * NULL if CONFIG_MEMCG is not configured.
What kind of locking is assumed here?
Are we protected against mmdrop() clearing out the mm?
> + */
> +__bpf_kfunc struct mem_cgroup *bpf_mm_get_mem_cgroup(struct mm_struct *mm)
> +{
> + return get_mem_cgroup_from_mm(mm);
> +}
> +
> +/**
> + * bpf_put_mem_cgroup - Release a memory cgroup obtained from bpf_mm_get_mem_cgroup()
> + * @memcg: The memory cgroup to release
> + */
> +__bpf_kfunc void bpf_put_mem_cgroup(struct mem_cgroup *memcg)
> +{
> +#ifdef CONFIG_MEMCG
> + if (!memcg)
> + return;
> + css_put(&memcg->css);
Feels weird to have an ifdef here but not elsewhere, maybe the whole thing
should be ifdef...?
Is there not a put equivalent for get_mem_cgroup_from_mm()? That is a bit weird.
Also do we now refrence the memcg global? That's pretty gross, could we not
actually implement such a helper?
Is it valid to do this also? Maybe cgroup people can chime in.
> +#endif
> +}
> +
> +__bpf_kfunc_end_defs();
> +
> +BTF_KFUNCS_START(bpf_thp_ids)
> +BTF_ID_FLAGS(func, bpf_mm_get_mem_cgroup, KF_TRUSTED_ARGS | KF_ACQUIRE | KF_RET_NULL)
> +BTF_ID_FLAGS(func, bpf_put_mem_cgroup, KF_RELEASE)
> +BTF_KFUNCS_END(bpf_thp_ids)
> +
> +static const struct btf_kfunc_id_set bpf_thp_set = {
> + .owner = THIS_MODULE,
> + .set = &bpf_thp_ids,
> +};
> +
> static int __init bpf_thp_ops_init(void)
> {
> - int err = register_bpf_struct_ops(&bpf_bpf_thp_ops, bpf_thp_ops);
> + int err;
> +
> + err = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_thp_set);
> + if (err) {
> + pr_err("bpf_thp: Failed to register kfunc sets (%d)\n", err);
> + return err;
> + }
>
> + err = register_bpf_struct_ops(&bpf_bpf_thp_ops, bpf_thp_ops);
> if (err)
> pr_err("bpf_thp: Failed to register struct_ops (%d)\n", err);
> return err;
Am again assuming this is legit BPF-wise :) Not my area... yet :>)
> --
> 2.47.3
>
