Re: [PATCH bpf-next v7 1/4] bpf: add bpf_get_cpu_cycles kfunc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux