From: "Björn Töpel" <bjorn@kernel.org>
To: Chen Pei <cp0613@linux.alibaba.com>,
ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
memxor@gmail.com, puranjay@kernel.org
Cc: ihor.solodrai@linux.dev, eddyz87@gmail.com, martin.lau@linux.dev,
song@kernel.org, yonghong.song@linux.dev, jolsa@kernel.org,
emil@etsalapatis.com, pulehui@huawei.com, pjw@kernel.org,
palmer@dabbelt.com, shuah@kernel.org, guoren@kernel.org,
bpf@vger.kernel.org, linux-riscv@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: Add support for indirect jumps
Date: Thu, 24 Sep 2026 17:03:13 +0200 [thread overview]
Message-ID: <87ld8qpu8e.fsf@all.your.base.are.belong.to.us> (raw)
In-Reply-To: <20260924102938-gotox-v2-style-question-cp0613@linux.alibaba.com>
Hey!
Chen Pei <cp0613@linux.alibaba.com> writes:
> Hi all,
>
> Thanks for the reviews. Patch 1 is a plain wording fix and will go into
> v2. For patch 2 I need your input on one thing before I respin: which
> form of the #endif comment you would prefer.
>
> Patch 1, on the comment above bpf_prog_update_insn_ptrs():
>
>> This isn't a bug, but could the comment say "shift ctx->offset[] by one"
>> rather than "shift the linfo array by one"? The jited_linfo was already
>> filled in by the bpf_prog_fill_jited_linfo() call above, and it is the
>> JIT's own offset table that is being rewritten here.
>
> Agreed. Fixed in v2.
>
> Patch 2, on the #endif marker in verifier_gotox.c:
>
>> [Severity: Low]
>> This isn't a bug, but does this newly introduced multi-line comment
>> follow the BPF subsystem style guide? The subsystem guidelines
>> explicitly require that multi-line comments have the opening '/*' on
>> its own line, rather than beginning text on the same line as the
>> opening marker.
>
> I would rather collapse it to a single line, but there is more than one
> way to spell it:
>
> a) 89 columns, clean under scripts/checkpatch.pl --strict:
>
> #endif /* __TARGET_ARCH_x86 || __TARGET_ARCH_arm64 || __TARGET_ARCH_powerpc || riscv64 */
>
> b) 101 columns, keeps every macro name verbatim, but checkpatch then
> reports "WARNING: line length of 101 exceeds 100 columns":
>
> #endif /* __TARGET_ARCH_x86 || __TARGET_ARCH_arm64 || __TARGET_ARCH_powerpc || __TARGET_ARCH_riscv */
>
> c) or a short marker that does not repeat the condition at all:
>
> #endif /* gotox-capable arch */
>
> I lean towards (a): tools/testing/selftests/bpf/progs/ otherwise only
> uses single-line #endif markers, and "riscv64" matches the guard, which
> is __TARGET_ARCH_riscv && __riscv_xlen == 64.
>
> I will send v2 with the patch 1 fix and whichever form you pick, together
> with any other feedback, so please let me know if you would like anything
> else changed. If nobody has a preference, I will go with (a) in a few
> days.
Maybe just (d):
| #endif /* gotox: x86, arm64, powerpc, riscv64 */
but (c) or (a) is fine as well, IMO.
Feel free to add for the series for your v2:
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Acked-by: Björn Töpel <bjorn@kernel.org>
prev parent reply other threads:[~2026-09-24 15:03 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-23 13:08 Chen Pei
2026-09-23 13:08 ` [PATCH bpf-next 1/2] " Chen Pei
2026-09-23 13:56 ` bot+bpf-ci
2026-09-23 13:08 ` [PATCH bpf-next 2/2] selftests/bpf: Enable gotox tests for riscv64 Chen Pei
2026-09-24 2:39 ` [PATCH bpf-next 0/2] bpf, riscv: Add support for indirect jumps Chen Pei
2026-09-24 15:03 ` Björn Töpel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ld8qpu8e.fsf@all.your.base.are.belong.to.us \
--to=bjorn@kernel.org \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=cp0613@linux.alibaba.com \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=emil@etsalapatis.com \
--cc=guoren@kernel.org \
--cc=ihor.solodrai@linux.dev \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=palmer@dabbelt.com \
--cc=pjw@kernel.org \
--cc=pulehui@huawei.com \
--cc=puranjay@kernel.org \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®