[PATCH bpf-next V2 0/3] Fix compiler warnings, looking for suggestions

[PATCH bpf-next V2 0/3] Fix compiler warnings, looking for suggestions

[Rafael Passos]

Hi,
This patchset has a few fixes to compiler warnings.
I am studying the BPF subsystem and wish to bring more tangible contributions.
I would appreciate receiving suggestions on things to investigate.
I also documented a bit in my blog. I could help with docs here, too.
https://rcpassos.me/post/linux-ebpf-understanding-kernel-level-mechanics
Thanks!

Changelog V1 -> V2:
- rebased all commits to updated for-next base
- removes new cases of the extra parameter for bpf_jit_binary_pack_finalize
- built and tested for ARM64
- sent the series for the test workflow:
  https://github.com/kernel-patches/bpf/pull/7198


Rafael Passos (3):
  bpf: remove unused parameter in bpf_jit_binary_pack_finalize
  bpf: remove unused parameter in __bpf_free_used_btfs
  bpf: remove redeclaration of new_n in bpf_verifier_vlog

 arch/arm64/net/bpf_jit_comp.c   | 3 +--
 arch/powerpc/net/bpf_jit_comp.c | 4 ++--
 arch/riscv/net/bpf_jit_core.c   | 5 ++---
 arch/x86/net/bpf_jit_comp.c     | 4 ++--
 include/linux/bpf.h             | 3 +--
 include/linux/filter.h          | 3 +--
 kernel/bpf/core.c               | 8 +++-----
 kernel/bpf/log.c                | 2 +-
 kernel/bpf/verifier.c           | 3 +--
 9 files changed, 14 insertions(+), 21 deletions(-)

-- 
2.45.2

[PATCH bpf-next V2 1/3] bpf: remove unused parameter in bpf_jit_binary_pack_finalize

[Rafael Passos]

Fixes a compiler warning. the bpf_jit_binary_pack_finalize function
was taking an extra bpf_prog parameter that went unused.
This removves it and updates the callers accordingly.

Signed-off-by: Rafael Passos <rafael@rcpassos.me>
---
 arch/arm64/net/bpf_jit_comp.c   | 3 +--
 arch/powerpc/net/bpf_jit_comp.c | 4 ++--
 arch/riscv/net/bpf_jit_core.c   | 5 ++---
 arch/x86/net/bpf_jit_comp.c     | 4 ++--
 include/linux/filter.h          | 3 +--
 kernel/bpf/core.c               | 3 +--
 6 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 720336d28856..6edaeafd1499 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1829,8 +1829,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 			prog->jited_len = 0;
 			goto out_free_hdr;
 		}
-		if (WARN_ON(bpf_jit_binary_pack_finalize(prog, ro_header,
-							 header))) {
+		if (WARN_ON(bpf_jit_binary_pack_finalize(ro_header, header))) {
 			/* ro_header has been freed */
 			ro_header = NULL;
 			prog = orig_prog;
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 984655419da5..2a36cc2e7e9e 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -225,7 +225,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 	fp->jited_len = proglen + FUNCTION_DESCR_SIZE;
 
 	if (!fp->is_func || extra_pass) {
-		if (bpf_jit_binary_pack_finalize(fp, fhdr, hdr)) {
+		if (bpf_jit_binary_pack_finalize(fhdr, hdr)) {
 			fp = org_fp;
 			goto out_addrs;
 		}
@@ -348,7 +348,7 @@ void bpf_jit_free(struct bpf_prog *fp)
 		 * before freeing it.
 		 */
 		if (jit_data) {
-			bpf_jit_binary_pack_finalize(fp, jit_data->fhdr, jit_data->hdr);
+			bpf_jit_binary_pack_finalize(jit_data->fhdr, jit_data->hdr);
 			kvfree(jit_data->addrs);
 			kfree(jit_data);
 		}
diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
index 0a96abdaca65..6de753c667f4 100644
--- a/arch/riscv/net/bpf_jit_core.c
+++ b/arch/riscv/net/bpf_jit_core.c
@@ -178,8 +178,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	prog->jited_len = prog_size - cfi_get_offset();
 
 	if (!prog->is_func || extra_pass) {
-		if (WARN_ON(bpf_jit_binary_pack_finalize(prog, jit_data->ro_header,
-							 jit_data->header))) {
+		if (WARN_ON(bpf_jit_binary_pack_finalize(jit_data->ro_header, jit_data->header))) {
 			/* ro_header has been freed */
 			jit_data->ro_header = NULL;
 			prog = orig_prog;
@@ -258,7 +257,7 @@ void bpf_jit_free(struct bpf_prog *prog)
 		 * before freeing it.
 		 */
 		if (jit_data) {
-			bpf_jit_binary_pack_finalize(prog, jit_data->ro_header, jit_data->header);
+			bpf_jit_binary_pack_finalize(jit_data->ro_header, jit_data->header);
 			kfree(jit_data);
 		}
 		hdr = bpf_jit_binary_pack_hdr(prog);
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 5159c7a22922..bd7e13602bf6 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -3363,7 +3363,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 			 *
 			 * Both cases are serious bugs and justify WARN_ON.
 			 */
-			if (WARN_ON(bpf_jit_binary_pack_finalize(prog, header, rw_header))) {
+			if (WARN_ON(bpf_jit_binary_pack_finalize(header, rw_header))) {
 				/* header has been freed */
 				header = NULL;
 				goto out_image;
@@ -3442,7 +3442,7 @@ void bpf_jit_free(struct bpf_prog *prog)
 		 * before freeing it.
 		 */
 		if (jit_data) {
-			bpf_jit_binary_pack_finalize(prog, jit_data->header,
+			bpf_jit_binary_pack_finalize(jit_data->header,
 						     jit_data->rw_header);
 			kvfree(jit_data->addrs);
 			kfree(jit_data);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index b02aea291b7e..dd41a93f06b2 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1129,8 +1129,7 @@ bpf_jit_binary_pack_alloc(unsigned int proglen, u8 **ro_image,
 			  struct bpf_binary_header **rw_hdr,
 			  u8 **rw_image,
 			  bpf_jit_fill_hole_t bpf_fill_ill_insns);
-int bpf_jit_binary_pack_finalize(struct bpf_prog *prog,
-				 struct bpf_binary_header *ro_header,
+int bpf_jit_binary_pack_finalize(struct bpf_binary_header *ro_header,
 				 struct bpf_binary_header *rw_header);
 void bpf_jit_binary_pack_free(struct bpf_binary_header *ro_header,
 			      struct bpf_binary_header *rw_header);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 1a6c3faa6e4a..f6951c33790d 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1174,8 +1174,7 @@ bpf_jit_binary_pack_alloc(unsigned int proglen, u8 **image_ptr,
 }
 
 /* Copy JITed text from rw_header to its final location, the ro_header. */
-int bpf_jit_binary_pack_finalize(struct bpf_prog *prog,
-				 struct bpf_binary_header *ro_header,
+int bpf_jit_binary_pack_finalize(struct bpf_binary_header *ro_header,
 				 struct bpf_binary_header *rw_header)
 {
 	void *ptr;
-- 
2.45.2

[PATCH bpf-next V2 2/3] bpf: remove unused parameter in __bpf_free_used_btfs

[Rafael Passos]

Fixes a compiler warning. The __bpf_free_used_btfs function
was taking an extra unused struct bpf_prog_aux *aux param

Signed-off-by: Rafael Passos <rafael@rcpassos.me>
---
 include/linux/bpf.h   | 3 +--
 kernel/bpf/core.c     | 5 ++---
 kernel/bpf/verifier.c | 3 +--
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f636b4998bf7..960780ef04e1 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2933,8 +2933,7 @@ bpf_probe_read_kernel_common(void *dst, u32 size, const void *unsafe_ptr)
 	return ret;
 }
 
-void __bpf_free_used_btfs(struct bpf_prog_aux *aux,
-			  struct btf_mod_pair *used_btfs, u32 len);
+void __bpf_free_used_btfs(struct btf_mod_pair *used_btfs, u32 len);
 
 static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
 						 enum bpf_prog_type type)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index f6951c33790d..ae2e1eeda0d4 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2742,8 +2742,7 @@ static void bpf_free_used_maps(struct bpf_prog_aux *aux)
 	kfree(aux->used_maps);
 }
 
-void __bpf_free_used_btfs(struct bpf_prog_aux *aux,
-			  struct btf_mod_pair *used_btfs, u32 len)
+void __bpf_free_used_btfs(struct btf_mod_pair *used_btfs, u32 len)
 {
 #ifdef CONFIG_BPF_SYSCALL
 	struct btf_mod_pair *btf_mod;
@@ -2760,7 +2759,7 @@ void __bpf_free_used_btfs(struct bpf_prog_aux *aux,
 
 static void bpf_free_used_btfs(struct bpf_prog_aux *aux)
 {
-	__bpf_free_used_btfs(aux, aux->used_btfs, aux->used_btf_cnt);
+	__bpf_free_used_btfs(aux->used_btfs, aux->used_btf_cnt);
 	kfree(aux->used_btfs);
 }
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index acc9dd830807..c703612e04f7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18619,8 +18619,7 @@ static void release_maps(struct bpf_verifier_env *env)
 /* drop refcnt of maps used by the rejected program */
 static void release_btfs(struct bpf_verifier_env *env)
 {
-	__bpf_free_used_btfs(env->prog->aux, env->used_btfs,
-			     env->used_btf_cnt);
+	__bpf_free_used_btfs(env->used_btfs, env->used_btf_cnt);
 }
 
 /* convert pseudo BPF_LD_IMM64 into generic BPF_LD_IMM64 */
-- 
2.45.2

[PATCH bpf-next V2 3/3] bpf: remove redeclaration of new_n in bpf_verifier_vlog

[Rafael Passos]

This new_n is defined in the start of this function.
Its value is overwritten by `new_n = min(n, log->len_total);`
a couple lines before my change,
rendering the shadow declaration unnecessary.

Signed-off-by: Rafael Passos <rafael@rcpassos.me>
---
 kernel/bpf/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
index 4bd8f17a9f24..10b2ed6995eb 100644
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -91,7 +91,7 @@ void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt,
 			goto fail;
 	} else {
 		u64 new_end, new_start;
-		u32 buf_start, buf_end, new_n;
+		u32 buf_start, buf_end;
 
 		new_end = log->end_pos + n;
 		if (new_end - log->start_pos >= log->len_total)
-- 
2.45.2

Re: [PATCH bpf-next V2 0/3] Fix compiler warnings, looking for suggestions

[Jiri Olsa]

On Fri, Jun 14, 2024 at 11:24:07PM -0300, Rafael Passos wrote:
> Hi,
> This patchset has a few fixes to compiler warnings.

curious, which compiler/setup displayed the warnings?

> I am studying the BPF subsystem and wish to bring more tangible contributions.
> I would appreciate receiving suggestions on things to investigate.
> I also documented a bit in my blog. I could help with docs here, too.
> https://rcpassos.me/post/linux-ebpf-understanding-kernel-level-mechanics
> Thanks!
> 
> Changelog V1 -> V2:
> - rebased all commits to updated for-next base
> - removes new cases of the extra parameter for bpf_jit_binary_pack_finalize
> - built and tested for ARM64
> - sent the series for the test workflow:
>   https://github.com/kernel-patches/bpf/pull/7198
> 
> 
> Rafael Passos (3):
>   bpf: remove unused parameter in bpf_jit_binary_pack_finalize
>   bpf: remove unused parameter in __bpf_free_used_btfs
>   bpf: remove redeclaration of new_n in bpf_verifier_vlog

lgtm, nice cleanup

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

jirka


> 
>  arch/arm64/net/bpf_jit_comp.c   | 3 +--
>  arch/powerpc/net/bpf_jit_comp.c | 4 ++--
>  arch/riscv/net/bpf_jit_core.c   | 5 ++---
>  arch/x86/net/bpf_jit_comp.c     | 4 ++--
>  include/linux/bpf.h             | 3 +--
>  include/linux/filter.h          | 3 +--
>  kernel/bpf/core.c               | 8 +++-----
>  kernel/bpf/log.c                | 2 +-
>  kernel/bpf/verifier.c           | 3 +--
>  9 files changed, 14 insertions(+), 21 deletions(-)
> 
> -- 
> 2.45.2
> 
> 

Re: [PATCH bpf-next V2 0/3] Fix compiler warnings, looking for suggestions

[Rafael Passos]

On 17/06/2024 05:45, Jiri Olsa wrote:
> On Fri, Jun 14, 2024 at 11:24:07PM -0300, Rafael Passos wrote:
>> Hi,
>> This patchset has a few fixes to compiler warnings.
> curious, which compiler/setup displayed the warnings?
>
It took me a few tries with different configs.
My most successful one was using gcc (14.1.1)
  make -j24 ARCH=x86_64 W=12 2>&1 | tee warnings.log
I dug through the Logs (with grep)looking for BPF and
  non macro expansion warnings.
Thanks!

>> I am studying the BPF subsystem and wish to bring more tangible contributions.
>> I would appreciate receiving suggestions on things to investigate.
>> I also documented a bit in my blog. I could help with docs here, too.
>> https://rcpassos.me/post/linux-ebpf-understanding-kernel-level-mechanics
>> Thanks!
>>
>> Changelog V1 -> V2:
>> - rebased all commits to updated for-next base
>> - removes new cases of the extra parameter for bpf_jit_binary_pack_finalize
>> - built and tested for ARM64
>> - sent the series for the test workflow:
>>    https://github.com/kernel-patches/bpf/pull/7198
>>
>>
>> Rafael Passos (3):
>>    bpf: remove unused parameter in bpf_jit_binary_pack_finalize
>>    bpf: remove unused parameter in __bpf_free_used_btfs
>>    bpf: remove redeclaration of new_n in bpf_verifier_vlog
> lgtm, nice cleanup
>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
>
> jirka
>
>
>>   arch/arm64/net/bpf_jit_comp.c   | 3 +--
>>   arch/powerpc/net/bpf_jit_comp.c | 4 ++--
>>   arch/riscv/net/bpf_jit_core.c   | 5 ++---
>>   arch/x86/net/bpf_jit_comp.c     | 4 ++--
>>   include/linux/bpf.h             | 3 +--
>>   include/linux/filter.h          | 3 +--
>>   kernel/bpf/core.c               | 8 +++-----
>>   kernel/bpf/log.c                | 2 +-
>>   kernel/bpf/verifier.c           | 3 +--
>>   9 files changed, 14 insertions(+), 21 deletions(-)
>>
>> -- 
>> 2.45.2
>>
>>

Re: [PATCH bpf-next V2 0/3] Fix compiler warnings, looking for suggestions

[Puranjay Mohan]

[-- Attachment #1: Type: text/plain, Size: 1470 bytes --]

Rafael Passos <rafael@rcpassos.me> writes:

> Hi,
> This patchset has a few fixes to compiler warnings.
> I am studying the BPF subsystem and wish to bring more tangible contributions.
> I would appreciate receiving suggestions on things to investigate.
> I also documented a bit in my blog. I could help with docs here, too.
> https://rcpassos.me/post/linux-ebpf-understanding-kernel-level-mechanics
> Thanks!
>
> Changelog V1 -> V2:
> - rebased all commits to updated for-next base
> - removes new cases of the extra parameter for bpf_jit_binary_pack_finalize
> - built and tested for ARM64
> - sent the series for the test workflow:
>   https://github.com/kernel-patches/bpf/pull/7198
>
>
> Rafael Passos (3):
>   bpf: remove unused parameter in bpf_jit_binary_pack_finalize
>   bpf: remove unused parameter in __bpf_free_used_btfs
>   bpf: remove redeclaration of new_n in bpf_verifier_vlog
>
>  arch/arm64/net/bpf_jit_comp.c   | 3 +--
>  arch/powerpc/net/bpf_jit_comp.c | 4 ++--
>  arch/riscv/net/bpf_jit_core.c   | 5 ++---
>  arch/x86/net/bpf_jit_comp.c     | 4 ++--
>  include/linux/bpf.h             | 3 +--
>  include/linux/filter.h          | 3 +--
>  kernel/bpf/core.c               | 8 +++-----
>  kernel/bpf/log.c                | 2 +-
>  kernel/bpf/verifier.c           | 3 +--
>  9 files changed, 14 insertions(+), 21 deletions(-)
>
> -- 
> 2.45.2

Acked-by: Puranjay Mohan <puranjay@kernel.org>

Thanks,
Puranjay

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

Re: [PATCH bpf-next V2 0/3] Fix compiler warnings, looking for suggestions

[patchwork-bot+netdevbpf]

Hello:

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

On Fri, 14 Jun 2024 23:24:07 -0300 you wrote:
> Hi,
> This patchset has a few fixes to compiler warnings.
> I am studying the BPF subsystem and wish to bring more tangible contributions.
> I would appreciate receiving suggestions on things to investigate.
> I also documented a bit in my blog. I could help with docs here, too.
> https://rcpassos.me/post/linux-ebpf-understanding-kernel-level-mechanics
> Thanks!
> 
> [...]

Here is the summary with links:
  - [bpf-next,V2,1/3] bpf: remove unused parameter in bpf_jit_binary_pack_finalize
    https://git.kernel.org/bpf/bpf-next/c/9919c5c98cb2
  - [bpf-next,V2,2/3] bpf: remove unused parameter in __bpf_free_used_btfs
    https://git.kernel.org/bpf/bpf-next/c/ab224b9ef7c4
  - [bpf-next,V2,3/3] bpf: remove redeclaration of new_n in bpf_verifier_vlog
    https://git.kernel.org/bpf/bpf-next/c/21ab4980e02d

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