Re: [PATCH v2 00/11] unwind, perf: sframe user space unwinding, deferred perf callchains

[Mathieu Desnoyers]

On 2024-09-16 02:15, Mathieu Desnoyers wrote:
> On 2024-09-16 20:15, Peter Zijlstra wrote:
> [...]
> > The point being that it is possible to wrap one CPU into the id space of
> > another CPU. It is not trivial, but someone who wants to can make it
> > happen.
>
> I agree that the overflow of the free-running counter bleeding into the
> CPU numbers is something we want to prevent. We don't care if this
> counter overflows after day/months/years for sake of correlation
> within a system call, but we do care about the fact that this
> free-running counter could be made to overflow after a very long
> time while the system runs, and then we reach a state where the
> CPU numbers are mixed up, which leads to short-term correlation
> issues.
>
> I would recommend this layout for this 64-bit value instead:
>
> low-bits: cpu number
> high-bits: free-running counter
>
> This way, we eliminate any carry from overflow into the cpu number bits.

Even better: AFAIR from the discussion I had with Steven and Josh, we intend
to have the cookie stored to/read from the task struct with interrupts off,
we can simply do:

struct stackwalk_cookie {
	uint64_t counter;	/* free running per-cpu counter value */
	int cpu;		/* cpu on which the counter was snapshot. */
};

So we don't have to deal with any overflow trickiness, there is no need for
bit-shifting, and we get a full 64-bit free-running counter independently of
the number of CPUs.

Thanks,

Mathieu



> Thanks,
>
> Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com