Re: [PATCH bpf-next v7 4/4] selftests/bpf: add usage example for cpu cycles kfuncs

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Nov 19, 2024 at 06:45:57AM -0800, Vadim Fedorenko wrote:
> On 19/11/2024 03:47, Peter Zijlstra wrote:
> > On Mon, Nov 18, 2024 at 10:52:45AM -0800, Vadim Fedorenko wrote:
> > 
> > > +int bpf_cpu_cycles(void)
> > > +{
> > > +	struct bpf_pidns_info pidns;
> > > +	__u64 start;
> > > +
> > > +	start = bpf_get_cpu_cycles();
> > > +	bpf_get_ns_current_pid_tgid(0, 0, &pidns, sizeof(struct bpf_pidns_info));
> > > +	cycles = bpf_get_cpu_cycles() - start;
> > > +	ns = bpf_cpu_cycles_to_ns(cycles);
> > > +	return 0;
> > > +}
> > 
> > Oh, the intent is to use that cycles_to_ns() on deltas. That wasn't at
> > all clear.
> 
> Yep, that's the main use case, it was discussed in the previous
> versions of the patchset.

Should bloody well be in the changelogs then. As is I'm tempted to NAK
the entire series because there is not a single word on WHY for any of
this.

> > Anyway, the above has more problems than just bad naming. TSC is
> > constant and not affected by DVFS, so depending on the DVFS state of
> > things your function will return wildly different readings.
> 
> Why should I care about DVFS? The use case is to measure the time spent
> in some code. If we replace it with bpf_ktime_get_ns(), it will also be
> affected by DVFS, and it's fine. We will be able to see the difference
> for different DVFS states.

Again, this goes to usage, why do you want this, what are you going to
do with the results?

Run-ro-run numbers will be absolutely useless because of DVFS.