Re: [PATCH bpf-next v4 3/7] bpf: move bpf_is_reg64() to fixups.c

[email protected]
Newsgroups org.kernel.vger.bpf
Message-ID <866402a885d28810f2dde7a7deceb995c88725278d40d2455f2ad97aaa925cf0@mail.kernel.org>
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 5f7843648189..d2ff416d7ad6 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -64,6 +64,96 @@ static int insn_def_regno(const struct bpf_insn *insn)
>  	}
>  }
>
> +/* This function is supposed to be used by the zero extension optimization
> + * code only. It returns TRUE if the source or destination register operates
> + * on 64-bit, otherwise return FALSE.
> + */

A subsystem pattern flags this as potentially concerning: the BPF
subsystem guide requires multi-line comments in kernel/bpf/ to follow
the modern style with the opening /* on its own line. This comment
puts text on the same line as the opening delimiter.

The guide states this applies even when surrounding code uses the old
style, and this comment's first line is modified by this patch ('the
following 32-bit optimization code only' -> 'the zero extension
optimization code only'), so it is a modified line rather than
untouched pre-existing text.

I noticed a later commit in the series (87fabf17dd53) rewrites this
comment into the compliant form, so the deviation is transient within
the series. Is it worth adjusting now, or acceptable to fix in the
later patch?

> +bool bpf_is_reg64(struct bpf_insn *insn,
> +		  u32 regno, struct bpf_reg_state *reg, enum bpf_reg_arg_type t)
> +{

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index be8818b9e640..abb325194168 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3058,96 +3058,6 @@ static void mark_stack_slots_scratched(struct bpf_verifier_env *env,
>  		mark_stack_slot_scratched(env, spi - i);
>  }
>
> -/* This function is supposed to be used by the following 32-bit optimization
> - * code only. It returns TRUE if the source or destination register operates
> - * on 64-bit, otherwise return FALSE.
> - */
> -bool bpf_is_reg64(struct bpf_insn *insn,
> -	      u32 regno, struct bpf_reg_state *reg, enum bpf_reg_arg_type t)
> -{

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31219172145
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.