On Wed, Dec 22, 2021 at 4:27 PM Yonghong Song [off-list ref] wrote:

On 12/22/21 1:39 PM, Andrii Nakryiko wrote:

quoted

Refactor PT_REGS macros definitions in  bpf_tracing.h to avoid excessive
duplication. We currently have classic PT_REGS_xxx() and CO-RE-enabled
PT_REGS_xxx_CORE(). We are about to add also _SYSCALL variants, which
would require excessive copying of all the per-architecture definitions.

Instead, separate architecture-specific field/register names from the
final macro that utilize them. That way for upcoming _SYSCALL variants
we'll be able to just define x86_64 exception and otherwise have one
common set of _SYSCALL macro definitions common for all architectures.

Cc: Kenta Tada <redacted>
Cc: Hengqi Chen <hengqi.chen@gmail.com>
Cc: Björn Töpel <bjorn@kernel.org>
Cc: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

I tried my best to compare old and new sources. Except "const volatile
struct user_pt_regs *" becomes "const struct user_pt_regs *", I didn't
spot any other semantic differences. Agree that "volatile" is not really
needed here. So

Right. I also started out with adding volatile for x86-64 case, but
that immediately triggered compilation warning in selftests, so I
suspect that other arches would be triggering warnings in some cases
due to volatile. So I dropped it everywhere because the load should
still happen due to the pointer dereference.

Acked-by: Yonghong Song <redacted>