Re: [PATCH bpf-next v3 2/3] bpf: struct_ops supports more than one page for trampolines.

[Martin KaFai Lau <martin.lau@linux.dev>]

On 2/23/24 7:03 PM, Kui-Feng Lee wrote:
> +static void bpf_struct_ops_map_free_image(struct bpf_struct_ops_map *st_map)
> +{
> +	int i;
> +	void *image;
> +
> +	bpf_jit_uncharge_modmem(PAGE_SIZE * st_map->image_pages_cnt);
> +	for (i = 0; i < st_map->image_pages_cnt; i++) {
> +		image = st_map->image_pages[i];
> +		arch_free_bpf_trampoline(image, PAGE_SIZE);
> +	}
> +	st_map->image_pages_cnt = 0;
> +}
> +

[ ... ]

> @@ -133,7 +128,8 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
>   	err = bpf_struct_ops_prepare_trampoline(tlinks, link,
>   						&st_ops->func_models[op_idx],
>   						&dummy_ops_test_ret_function,
> -						image, image + PAGE_SIZE);
> +						&image, &image_off,
> +						true);
>   	if (err < 0)
>   		goto out;
>   
> @@ -147,6 +143,8 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
>   		err = -EFAULT;
>   out:
>   	kfree(args);
> +	if (image)
> +		bpf_jit_uncharge_modmem(PAGE_SIZE);
>   	arch_free_bpf_trampoline(image, PAGE_SIZE);

It seems my last reply on v2 has crossed over with v3.

The bpf_struct_ops_free_trampoline() highlighted in my last reply should
address your concern in v2 that the caller needs to remember
the bpf_jit_uncharge_modmem here.

I think the trampoline alloc(aka prepare here)/free pair that you also
suggested in v2 discussion is a nice match here and work as a
charge+alloc and uncharge+free pair.

>   	if (link)
>   		bpf_link_put(&link->link);