Re: [PATCH bpf-next v3 06/15] bpf: Add prog_list_init_item(), prog_list_replace_item(), and prog_list_id()

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Mon Jul 6, 2026 at 1:19 PM EDT, Amery Hung wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
>
> Add three helpers to abstract operations on a bpf_prog_list entry.
>
> Right now, bpf_prog_array_item is initialized from prog_list_prog(pl),
> which returns either pl->prog or pl->link->link.prog. This will not work
> when struct_ops is attached to a cgroup because the attachment is backed
> by a struct_ops map instead of a BPF prog.
>
> The same applies to __cgroup_bpf_query(). Instead of always copying a
> prog id to userspace, struct_ops cgroup attachment will need to copy the
> struct_ops map id.
>
> Refactor bpf_prog_array_item initialization into prog_list_init_item()
> and prog_list_replace_item(), and refactor id lookup into prog_list_id().
> These helpers will be extended to support pl->link->map in a later patch.
>
> This is a no-op change.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

Followup on prog_list_prog() since I hadn't realized it was moved to the
helpers in this patchset: Maybe we can just make it not return NULL
since it's impossible instead of adding error handling to minimize
churn.

>
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
>  kernel/bpf/cgroup.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index b100c04cb9c8..b43f0bff184c 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -399,6 +399,22 @@ static struct bpf_prog *prog_list_prog(struct bpf_prog_list *pl)
>  	return NULL;
>  }
>  
> +static void prog_list_init_item(struct bpf_prog_list *pl, struct bpf_prog_array_item *item)
> +{
> +	item->prog = prog_list_prog(pl);
> +	bpf_cgroup_storages_assign(item->cgroup_storage, pl->storage);
> +}
> +
> +static void prog_list_replace_item(struct bpf_prog_list *pl, struct bpf_prog_array_item *item)
> +{
> +	WRITE_ONCE(item->prog, pl->link->link.prog);
> +}
> +
> +static u32 prog_list_id(struct bpf_prog_list *pl)
> +{
> +	return prog_list_prog(pl)->aux->id;
> +}
> +
>  /* count number of elements in the list.
>   * it's slow but the list cannot be long
>   */
> @@ -492,9 +508,7 @@ static int compute_effective_progs(struct cgroup *cgrp,
>  				item = &progs->items[fstart];
>  				fstart++;
>  			}
> -			item->prog = prog_list_prog(pl);
> -			bpf_cgroup_storages_assign(item->cgroup_storage,
> -						   pl->storage);
> +			prog_list_init_item(pl, item);
>  			cnt++;
>  		}
>  
> @@ -1015,7 +1029,7 @@ static void replace_effective_prog(struct cgroup *cgrp,
>  				desc->bpf.effective[atype],
>  				lockdep_is_held(&cgroup_mutex));
>  		item = &progs->items[pos];
> -		WRITE_ONCE(item->prog, pl->link->link.prog);
> +		prog_list_replace_item(pl, item);
>  	}
>  }
>  
> @@ -1318,15 +1332,13 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
>  		} else {
>  			struct hlist_head *progs;
>  			struct bpf_prog_list *pl;
> -			struct bpf_prog *prog;
>  			u32 id;
>  
>  			progs = &cgrp->bpf.progs[atype];
>  			cnt = min_t(int, prog_list_length(progs, NULL), total_cnt);
>  			i = 0;
>  			hlist_for_each_entry(pl, progs, node) {
> -				prog = prog_list_prog(pl);
> -				id = prog->aux->id;
> +				id = prog_list_id(pl);
>  				if (copy_to_user(prog_ids + i, &id, sizeof(id)))
>  					return -EFAULT;
>  				if (++i == cnt)