nvproxy: expose CLOCK_MONOTONIC_RAW as a distinct clock for capProfiling - #14002
nvproxy: expose CLOCK_MONOTONIC_RAW as a distinct clock for capProfiling#14002luiscape wants to merge 1 commit into
Conversation
f7395ae to
9be64c1
Compare
|
@aaltinaydev thank you for the review. I added a warning and panic. |
|
We may be iterating on this PR. Not ready to pull yet! |
9be64c1 to
98bcf81
Compare
98bcf81 to
055c868
Compare
ayushr2
left a comment
There was a problem hiding this comment.
Thanks Luis!! Just nits.
| // clock_gettime(CLOCK_MONOTONIC_RAW). | ||
| int ClockMonotonicRaw(struct timespec* ts) { | ||
| if (get_params()->monotonic_raw_syscall) { | ||
| return sys_clock_gettime(CLOCK_MONOTONIC_RAW, ts); |
There was a problem hiding this comment.
Profilers timestamp at high rates inside the process being measured, so a syscall per timestamp will probably hurt and give poor timestamp measurements. VDSO exists to avoid the syscall context switch.
Can we serve this from the VDSO instead of falling back to the syscall? The param page has plenty of room (currently seq + 8 u64s in a 4KiB page), and all clocks calibrate against the same TSC, so it's mostly mechanical:
- add monotonicRaw{Ready,BaseCycles,BaseRef,Frequency} to vdsoParams and mirror them in struct params
- publish them in Timekeeper.update(), without monotonicOffset
- make ClockMonotonicRaw() a copy of ClockMonotonic() reading the raw fields
Clocks.Update() would need to return the raw params too. Six return values is a lot; maybe fold them into a struct.
One thing to be careful about: we need to distinguish "raw disabled" from "raw enabled but not yet calibrated". If we only check ready and fall back to the syscall, every non-profiling sandbox regresses from today's free VDSO alias to a syscall. So !enabled should call ClockMonotonic() directly, and only enabled && !ready should hit the syscall:
int ClockMonotonicRaw(struct timespec* ts) {
// ...seqcount loop reading enabled, ready, base_ref, base_cycles, frequency...
if (!enabled) return ClockMonotonic(ts); // in-VDSO alias, as today
if (!ready) return sys_clock_gettime(CLOCK_MONOTONIC_RAW, ts);
// ...same delta_cycles/cycles_to_ns computation...
}
I think accuracy should not be impacted, since the syscall path uses the same calibrated parameters the VDSO would recompute.
055c868 to
9c529bd
Compare
GPU profilers (Nsight Systems/CUPTI) build their trace timeline from CLOCK_MONOTONIC_RAW. gVisor serves CLOCK_MONOTONIC_RAW as an alias of CLOCK_MONOTONIC, this will drift from CLOCK_MONOTONIC_RAW breaking profiling. When nvproxy is enabled with `capProfiling`, this change exposes CLOCK_MONOTONIC_RAW as a clock distinct from CLOCK_MONOTONIC that tracks the host's CLOCK_MONOTONIC_RAW (absolute, unadjusted). CLOCK_MONOTONIC is unchanged. When profiling is disabled, CLOCK_MONOTONIC_RAW continues to alias CLOCK_MONOTONIC as before.
9c529bd to
b12dd08
Compare
GPU profilers (Nsight Systems/CUPTI) build their trace timeline from CLOCK_MONOTONIC_RAW. gVisor serves CLOCK_MONOTONIC_RAW as an alias of CLOCK_MONOTONIC, this will drift from CLOCK_MONOTONIC_RAW breaking profiling.
When nvproxy is enabled with
capProfiling, this change exposes CLOCK_MONOTONIC_RAW as a clock distinct from CLOCK_MONOTONIC that tracks the host's CLOCK_MONOTONIC_RAW (absolute, unadjusted). CLOCK_MONOTONIC is unchanged. When profiling is disabled, CLOCK_MONOTONIC_RAW continues to alias CLOCK_MONOTONIC as before.This is an example program that won't store kernel traces without this change:
nsys profile --trace=cuda --sample=none --cpuctxsw=none \ --export=sqlite -o /out/torch \ python3 -c 'import torch; a=torch.randn(1024,1024,device="cuda"); (a@a).sum().item()'One can verify that's the case by looking at the resulting SQLite database:
sqlite3 /tmp/torch.sqlite "SELECT count(*) FROM CUPTI_ACTIVITY_KIND_KERNEL"Assisted-by: Claude