[PATCH bpf-next v2] bpf: Only allocate one bpf_mem_cache for bpf_cpumask_ma

[PATCH bpf-next v2] bpf: Only allocate one bpf_mem_cache for bpf_cpumask_ma

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

The size of bpf_cpumask is fixed, so there is no need to allocate many
bpf_mem_caches for bpf_cpumask_ma, just one bpf_mem_cache is enough.
Also add comments for bpf_mem_alloc_init() in bpf_mem_alloc.h to prevent
future miuse.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
v2: fix typo (forget to regenerate the patch after testing)
v1: https://lore.kernel.org/bpf/7736864d-af8d-4f74-086b-0ec125aae2a6@huawei.com/T/#t

 include/linux/bpf_mem_alloc.h | 7 +++++++
 kernel/bpf/cpumask.c          | 6 +++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/linux/bpf_mem_alloc.h b/include/linux/bpf_mem_alloc.h
index 3e164b8efaa9..a7104af61ab4 100644
--- a/include/linux/bpf_mem_alloc.h
+++ b/include/linux/bpf_mem_alloc.h
@@ -14,6 +14,13 @@ struct bpf_mem_alloc {
 	struct work_struct work;
 };
 
+/* 'size != 0' is for bpf_mem_alloc which manages fixed-size objects.
+ * Alloc and free are done with bpf_mem_cache_{alloc,free}().
+ *
+ * 'size = 0' is for bpf_mem_alloc which manages many fixed-size objects.
+ * Alloc and free are done with bpf_mem_{alloc,free}() and the size of
+ * the returned object is given by the size argument of bpf_mem_alloc().
+ */
 int bpf_mem_alloc_init(struct bpf_mem_alloc *ma, int size, bool percpu);
 void bpf_mem_alloc_destroy(struct bpf_mem_alloc *ma);
 
diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
index 52b981512a35..2b3fbbfebdc5 100644
--- a/kernel/bpf/cpumask.c
+++ b/kernel/bpf/cpumask.c
@@ -55,7 +55,7 @@ __bpf_kfunc struct bpf_cpumask *bpf_cpumask_create(void)
 	/* cpumask must be the first element so struct bpf_cpumask be cast to struct cpumask. */
 	BUILD_BUG_ON(offsetof(struct bpf_cpumask, cpumask) != 0);
 
-	cpumask = bpf_mem_alloc(&bpf_cpumask_ma, sizeof(*cpumask));
+	cpumask = bpf_mem_cache_alloc(&bpf_cpumask_ma);
 	if (!cpumask)
 		return NULL;
 
@@ -123,7 +123,7 @@ __bpf_kfunc void bpf_cpumask_release(struct bpf_cpumask *cpumask)
 
 	if (refcount_dec_and_test(&cpumask->usage)) {
 		migrate_disable();
-		bpf_mem_free(&bpf_cpumask_ma, cpumask);
+		bpf_mem_cache_free(&bpf_cpumask_ma, cpumask);
 		migrate_enable();
 	}
 }
@@ -468,7 +468,7 @@ static int __init cpumask_kfunc_init(void)
 		},
 	};
 
-	ret = bpf_mem_alloc_init(&bpf_cpumask_ma, 0, false);
+	ret = bpf_mem_alloc_init(&bpf_cpumask_ma, sizeof(struct bpf_cpumask), false);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &cpumask_kfunc_set);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &cpumask_kfunc_set);
 	return  ret ?: register_btf_id_dtor_kfuncs(cpumask_dtors,
-- 
2.29.2

Re: [PATCH bpf-next v2] bpf: Only allocate one bpf_mem_cache for bpf_cpumask_ma

[Jiri Olsa]

On Thu, Feb 16, 2023 at 10:48:21AM +0800, Hou Tao wrote:
> From: Hou Tao <houtao1@huawei.com>
> 
> The size of bpf_cpumask is fixed, so there is no need to allocate many
> bpf_mem_caches for bpf_cpumask_ma, just one bpf_mem_cache is enough.
> Also add comments for bpf_mem_alloc_init() in bpf_mem_alloc.h to prevent
> future miuse.
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> v2: fix typo (forget to regenerate the patch after testing)
> v1: https://lore.kernel.org/bpf/7736864d-af8d-4f74-086b-0ec125aae2a6@huawei.com/T/#t

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> 
>  include/linux/bpf_mem_alloc.h | 7 +++++++
>  kernel/bpf/cpumask.c          | 6 +++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/bpf_mem_alloc.h b/include/linux/bpf_mem_alloc.h
> index 3e164b8efaa9..a7104af61ab4 100644
> --- a/include/linux/bpf_mem_alloc.h
> +++ b/include/linux/bpf_mem_alloc.h
> @@ -14,6 +14,13 @@ struct bpf_mem_alloc {
>  	struct work_struct work;
>  };
>  
> +/* 'size != 0' is for bpf_mem_alloc which manages fixed-size objects.
> + * Alloc and free are done with bpf_mem_cache_{alloc,free}().
> + *
> + * 'size = 0' is for bpf_mem_alloc which manages many fixed-size objects.
> + * Alloc and free are done with bpf_mem_{alloc,free}() and the size of
> + * the returned object is given by the size argument of bpf_mem_alloc().
> + */
>  int bpf_mem_alloc_init(struct bpf_mem_alloc *ma, int size, bool percpu);
>  void bpf_mem_alloc_destroy(struct bpf_mem_alloc *ma);
>  
> diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
> index 52b981512a35..2b3fbbfebdc5 100644
> --- a/kernel/bpf/cpumask.c
> +++ b/kernel/bpf/cpumask.c
> @@ -55,7 +55,7 @@ __bpf_kfunc struct bpf_cpumask *bpf_cpumask_create(void)
>  	/* cpumask must be the first element so struct bpf_cpumask be cast to struct cpumask. */
>  	BUILD_BUG_ON(offsetof(struct bpf_cpumask, cpumask) != 0);
>  
> -	cpumask = bpf_mem_alloc(&bpf_cpumask_ma, sizeof(*cpumask));
> +	cpumask = bpf_mem_cache_alloc(&bpf_cpumask_ma);
>  	if (!cpumask)
>  		return NULL;
>  
> @@ -123,7 +123,7 @@ __bpf_kfunc void bpf_cpumask_release(struct bpf_cpumask *cpumask)
>  
>  	if (refcount_dec_and_test(&cpumask->usage)) {
>  		migrate_disable();
> -		bpf_mem_free(&bpf_cpumask_ma, cpumask);
> +		bpf_mem_cache_free(&bpf_cpumask_ma, cpumask);
>  		migrate_enable();
>  	}
>  }
> @@ -468,7 +468,7 @@ static int __init cpumask_kfunc_init(void)
>  		},
>  	};
>  
> -	ret = bpf_mem_alloc_init(&bpf_cpumask_ma, 0, false);
> +	ret = bpf_mem_alloc_init(&bpf_cpumask_ma, sizeof(struct bpf_cpumask), false);
>  	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &cpumask_kfunc_set);
>  	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &cpumask_kfunc_set);
>  	return  ret ?: register_btf_id_dtor_kfuncs(cpumask_dtors,
> -- 
> 2.29.2
> 

Re: [PATCH bpf-next v2] bpf: Only allocate one bpf_mem_cache for bpf_cpumask_ma

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Thu, 16 Feb 2023 10:48:21 +0800 you wrote:
> From: Hou Tao <houtao1@huawei.com>
> 
> The size of bpf_cpumask is fixed, so there is no need to allocate many
> bpf_mem_caches for bpf_cpumask_ma, just one bpf_mem_cache is enough.
> Also add comments for bpf_mem_alloc_init() in bpf_mem_alloc.h to prevent
> future miuse.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] bpf: Only allocate one bpf_mem_cache for bpf_cpumask_ma
    https://git.kernel.org/bpf/bpf-next/c/5d5de3a431d8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html