On 19/11/2024 08:17, Peter Zijlstra wrote:
On Tue, Nov 19, 2024 at 06:29:09AM -0800, Vadim Fedorenko wrote:
On 19/11/2024 03:18, Peter Zijlstra wrote:
On Mon, Nov 18, 2024 at 10:52:42AM -0800, Vadim Fedorenko wrote:
@@ -2094,6 +2094,13 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
int err;
+ if (imm32 == BPF_CALL_IMM(bpf_get_cpu_cycles)) {
+ if (cpu_feature_enabled(X86_FEATURE_LFENCE_RDTSC))
+ EMIT3(0x0F, 0xAE, 0xE8);
+ EMIT2(0x0F, 0x31);
+ break;
+ }
TSC != cycles. Naming is bad.
Any suggestions?
JIT for other architectures will come after this one is merged and some
of them will be using cycles, so not too far away form the truth..
bpf_get_time_stamp() ?
bpf_get_counter() ?
Well, we have already been somewhere nearby these names [1].
[1]
https://lore.kernel.org/bpf/CAEf4BzaBNNCYaf9a4oHsB2AzYyc6JCWXpHx6jk22Btv=UAgX4A@xxxxxxxxxxxxxx/
bpf_get_time_stamp() doesn't really explain that the actual timestamp
will be provided by CPU hardware.
bpf_get_counter() is again too general, doesn't provide any information
about what type of counter will be returned. The more specific name,
bpf_get_cycles_counter(), was also discussed in v3 (accidentally, it
didn't reach mailing list). The quote of feedback from Andrii is:
Bikeshedding time, but let's be consistently slightly verbose, but
readable. Give nwe have bpf_get_cpu_cycles_counter (which maybe we
should shorten to "bpf_get_cpu_cycles()"), we should call this
something like "bpf_cpu_cycles_to_ns()".
It might make a bit more sense to name it bpf_get_cpu_counter(), but it
still looks too general.
Honestly, I'm not a fan of renaming functions once again, I would let
Andrii to vote for naming.