On Tue, Nov 19, 2024 at 10:03 AM Vadim Fedorenko
<vadim.fedorenko@xxxxxxxxx> wrote:
>
> 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.
Let's go with bpf_get_cpu_time_counter() and bpf_cpu_time_counter_to_ns().
