Re: [PATCH bpf-next v3 02/17] bpf: allow attaching struct_ops to cgroups

[Josh Don]

Hi Roman,

On Mon, Jan 26, 2026 at 6:50 PM Roman Gushchin <roman.gushchin@xxxxxxxxx> wrote:
>
> Introduce an ability to attach bpf struct_ops'es to cgroups.
>
[snip]
>  struct bpf_struct_ops_value {
>         struct bpf_struct_ops_common_value common;
> @@ -1220,6 +1222,10 @@ static void bpf_struct_ops_map_link_dealloc(struct bpf_link *link)
>                 st_map->st_ops_desc->st_ops->unreg(&st_map->kvalue.data, link);
>                 bpf_map_put(&st_map->map);
>         }
> +
> +       if (st_link->cgroup)
> +               cgroup_bpf_detach_struct_ops(st_link->cgroup, st_link);
> +

I was worried about concurrency with cgroup ops until I saw
cgroup_bpf_detach_struct_ops() takes cgroup_lock() internally (since
you take it inline sometimes below I falsely assumed it wasn't
present). In any case, I'm wondering why you need to pass in the
cgroup pointer to cgroup_bpf_detach_struct_ops() at all, rather than
just the link?


> @@ -1357,8 +1386,12 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
>         struct bpf_link_primer link_primer;
>         struct bpf_struct_ops_map *st_map;
>         struct bpf_map *map;
> +       struct cgroup *cgrp;
>         int err;
>
> +       if (attr->link_create.flags & ~BPF_F_CGROUP_FD)
> +               return -EINVAL;
> +
>         map = bpf_map_get(attr->link_create.map_fd);
>         if (IS_ERR(map))
>                 return PTR_ERR(map);
> @@ -1378,11 +1411,26 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
>         bpf_link_init(&link->link, BPF_LINK_TYPE_STRUCT_OPS, &bpf_struct_ops_map_lops, NULL,
>                       attr->link_create.attach_type);
>
> +       init_waitqueue_head(&link->wait_hup);
> +
> +       if (attr->link_create.flags & BPF_F_CGROUP_FD) {
> +               cgrp = cgroup_get_from_fd(attr->link_create.target_fd);
> +               if (IS_ERR(cgrp)) {
> +                       err = PTR_ERR(cgrp);
> +                       goto err_out;
> +               }
> +               link->cgroup = cgrp;
> +               err = cgroup_bpf_attach_struct_ops(cgrp, link);

We have to be careful at this point. cgroup release could now occur
concurrently which would clear link->cgroup. Maybe worth a comment
here since this is a bit subtle.

> +               if (err) {
> +                       cgroup_put(cgrp);
> +                       link->cgroup = NULL;
> +                       goto err_out;
> +               }
> +       }