diff --git a/gpuprobe/consumer.go b/gpuprobe/consumer.go index e558685..a19aba5 100644 --- a/gpuprobe/consumer.go +++ b/gpuprobe/consumer.go @@ -437,6 +437,12 @@ type Config struct { // an offer that would pass it is refused and counted, again in // CubinsRejectedTooLarge. CubinTotalBytes int64 + + // EventRingBytes sizes the BPF event ringbuf. Zero keeps the + // compiled-in 4 MB. Must be a power of two and a multiple of the page + // size -- the kernel rejects anything else, so it is rounded up and + // validated here rather than failing deep in the verifier. + EventRingBytes int } const ( @@ -1703,6 +1709,28 @@ func Attach(cfg Config) (c *Consumer, err error) { } }() + // Size the event ringbuf before load. The compiled-in 4 MB holds a + // default-period capture comfortably, but the event rate scales with + // 1/PERFAGENT_GPU_SAMPLE_PERIOD: at period=1 a PyTorch step rate that + // loses ~5% of records at the default loses ~90%, nearly all of it + // kernel_dropped -- batches the ringbuf had no room for. Those carry the + // kernel activity records that ARE the GPU time, so the profile shrinks + // rather than merely losing attribution. Raising the buffer is what lets + // a low period mean "more attribution" instead of "less profile". + if c.cfg.EventRingBytes > 0 { + m, ok := spec.Maps["events"] + if !ok { + err = fmt.Errorf("event ringbuf map %q not in spec", "events") + return + } + size, rerr := roundRingBytes(c.cfg.EventRingBytes) + if rerr != nil { + err = rerr + return + } + m.MaxEntries = size + } + if err = spec.LoadAndAssign(&c.objs, nil); err != nil { err = fmt.Errorf("load gpu usdt objects: %w", err) return @@ -3376,3 +3404,23 @@ func (c *Consumer) Close() error { errs = append(errs, c.objs.Close()) return errors.Join(errs...) } + +// roundRingBytes rounds n up to a power of two that is also a multiple of the +// page size, which is what BPF_MAP_TYPE_RINGBUF requires of max_entries. The +// kernel rejects a bad size at load time with a bare EINVAL, far from the +// knob that caused it, so the rounding and the bound both happen here. +func roundRingBytes(n int) (uint32, error) { + if n <= 0 { + return 0, fmt.Errorf("event ringbuf size must be positive, got %d", n) + } + const maxRingBytes = 1 << 30 // 1 GiB of locked memory is already absurd + if n > maxRingBytes { + return 0, fmt.Errorf("event ringbuf size %d exceeds the %d-byte cap", n, maxRingBytes) + } + page := os.Getpagesize() + size := page + for size < n { + size <<= 1 + } + return uint32(size), nil +} diff --git a/gpuprobe/ringbytes_test.go b/gpuprobe/ringbytes_test.go new file mode 100644 index 0000000..733d44a --- /dev/null +++ b/gpuprobe/ringbytes_test.go @@ -0,0 +1,55 @@ +package gpuprobe + +import ( + "os" + "testing" +) + +// TestRoundRingBytes pins the two properties BPF_MAP_TYPE_RINGBUF requires of +// max_entries -- a power of two, and a multiple of the page size -- plus the +// bounds. A size that satisfies neither is rejected by the verifier with a +// bare EINVAL far from the knob that set it, which is what this rounding is +// for. +func TestRoundRingBytes(t *testing.T) { + page := os.Getpagesize() + + t.Run("rounds up to a power-of-two page multiple", func(t *testing.T) { + for _, in := range []int{1, page - 1, page, page + 1, 4 << 20, (4 << 20) + 1, 100 << 20} { + got, err := roundRingBytes(in) + if err != nil { + t.Fatalf("roundRingBytes(%d): unexpected error %v", in, err) + } + n := int(got) + if n < in { + t.Errorf("roundRingBytes(%d) = %d, rounded DOWN", in, n) + } + if n&(n-1) != 0 { + t.Errorf("roundRingBytes(%d) = %d, not a power of two", in, n) + } + if n%page != 0 { + t.Errorf("roundRingBytes(%d) = %d, not a multiple of page %d", in, n, page) + } + // Rounding up must not overshoot past the next power of two. + if in > page && n >= in*2 { + t.Errorf("roundRingBytes(%d) = %d, overshot", in, n) + } + } + }) + + t.Run("rejects non-positive", func(t *testing.T) { + for _, in := range []int{0, -1, -(4 << 20)} { + if _, err := roundRingBytes(in); err == nil { + t.Errorf("roundRingBytes(%d): want error, got nil", in) + } + } + }) + + t.Run("rejects absurd", func(t *testing.T) { + if _, err := roundRingBytes((1 << 30) + 1); err == nil { + t.Error("roundRingBytes(1GiB+1): want error, got nil") + } + if _, err := roundRingBytes(1 << 30); err != nil { + t.Errorf("roundRingBytes(1GiB): want ok, got %v", err) + } + }) +} diff --git a/gpuprofile/profiler.go b/gpuprofile/profiler.go index 43e136e..12523c4 100644 --- a/gpuprofile/profiler.go +++ b/gpuprofile/profiler.go @@ -71,6 +71,10 @@ type Config struct { // to a busy process overruns them and loses GPU time outright. Zero means // DefaultDrainEvery. DrainEvery time.Duration + + // EventRingBytes sizes the BPF event ringbuf. Zero keeps the + // compiled-in default, which is sized for a default-period capture. + EventRingBytes int } // DefaultDrainEvery matches cmd/gpu-cuda-profile's default. @@ -136,6 +140,7 @@ func New(cfg Config) (*Profiler, error) { Symbolizer: cfg.Symbolizer, Modules: store, KeepInstrumentationFrames: cfg.KeepInstrumentationFrames, + EventRingBytes: cfg.EventRingBytes, }) if err != nil { return nil, fmt.Errorf("gpuprofile: attach to %s: %w", cfg.ShimPath, err) @@ -241,6 +246,15 @@ func (p *Profiler) Samples() int { return p.samples } +// Stats reports the consumer's ingestion counters: what arrived off the +// ringbuf and what was lost on the way. The profile alone cannot show this -- +// a capture that dropped most of its launches still writes a well-formed +// profile, just a smaller one -- so the caller needs the counters to tell a +// quiet GPU from a lossy capture. +func (p *Profiler) Stats() gpuprobe.Stats { + return p.consumer.Stats() +} + // stop cancels the run loop once and waits for the final drain. func (p *Profiler) stop() { p.mu.Lock() diff --git a/main.go b/main.go index 038c567..4cb6174 100644 --- a/main.go +++ b/main.go @@ -42,7 +42,13 @@ var ( flagGPUShim = flag.String("gpu-shim", "", "The CUPTI adapter the target loaded. Must be the SAME FILE it mapped: the uprobe attaches "+ "by inode, so identical bytes at another path is a different target and nothing is captured") - flagGPUOutput = flag.String("gpu-output", "", "Output path for GPU profile (default: auto-generated)") + flagGPUOutput = flag.String("gpu-output", "", "Output path for GPU profile (default: auto-generated)") + flagGPURingBytes = flag.Int("gpu-ring-bytes", 0, + "Size of the BPF event ringbuf for --gpu, in bytes (rounded up to a "+ + "power-of-two page multiple; 0 keeps the 4MB default). Lowering "+ + "PERFAGENT_GPU_SAMPLE_PERIOD in the target multiplies the event rate, "+ + "and the records that overflow carry the GPU time itself, so an "+ + "undersized buffer SHRINKS the profile. Check the 'gpu: LOST' line") flagPMUOutput = flag.String("pmu-output", "", "Output path for PMU metrics (default: stdout)") flagFlamegraph = flag.String("flamegraph-output", "", "Also write a self-contained interactive HTML flame graph of the profile. "+ @@ -258,6 +264,9 @@ func buildOptions() []perfagent.Option { perfagent.WithGPU(*flagGPUShim), perfagent.WithGPUProfilePath(outputPath), ) + if *flagGPURingBytes > 0 { + opts = append(opts, perfagent.WithGPUEventRingBytes(*flagGPURingBytes)) + } if path := flamegraphPath(*flagFlamegraph, "gpu"); path != "" { opts = append(opts, perfagent.WithGPUFlamegraph(path)) diff --git a/perfagent/agent.go b/perfagent/agent.go index 1c9e6ac..82e18c0 100644 --- a/perfagent/agent.go +++ b/perfagent/agent.go @@ -622,6 +622,7 @@ func (a *Agent) Start(ctx context.Context) error { Symbolizer: a.symbolizer, PCSampling: a.config.GPUPCSampling, KeepInstrumentationFrames: a.config.GPUKeepInstrumentationFrames, + EventRingBytes: a.config.GPUEventRingBytes, }) if err != nil { a.cleanup() @@ -858,9 +859,43 @@ func (a *Agent) logDebuginfodStats() { st.FileModeAddrs, st.FileModeFetchFails, st.FileModeLocalHits, st.FileModeParseFails) } +// logGPUStats prints the GPU consumer's ingestion counters. +// +// Same shape as issue #109: the counters existed in gpuprobe.Stats and +// nothing printed them, so a capture that lost most of its GPU work still +// reported only "GPU profile written to ... (N samples)" and looked like a +// success. N alone cannot distinguish a quiet GPU from a lossy capture. +// +// The loss counters are called out separately from the volume ones because +// a non-zero value in any of them means the profile under-reports: samples +// the kernel could not deliver, and stacks evicted from the side table +// before their kernel record arrived, are both GPU work that happened and +// is not in the file. +func (a *Agent) logGPUStats() { + if a.gpuProfiler == nil { + return + } + st := a.gpuProfiler.Stats() + log.Printf("perf-agent: gpu: batches=%d records=%d sampled_launches=%d "+ + "stacks_resolved=%d stacks_attached=%d", + st.Batches, st.Records, st.SampledLaunches, + st.StacksResolved, st.StacksAttached) + if lost := st.KernelDropped + st.StacksEvicted + st.SequenceGaps + + st.StacksMissing + st.SinkRejected; lost > 0 { + log.Printf("perf-agent: gpu: LOST %d record(s): kernel_dropped=%d "+ + "stacks_evicted=%d sequence_gaps=%d stacks_missing=%d sink_rejected=%d "+ + "-- the profile under-reports GPU work. A smaller "+ + "PERFAGENT_GPU_SAMPLE_PERIOD raises the event rate and makes this "+ + "worse; raise the period or shorten the capture", + lost, st.KernelDropped, st.StacksEvicted, st.SequenceGaps, + st.StacksMissing, st.SinkRejected) + } +} + // cleanup releases profiler resources. func (a *Agent) cleanup() { a.logDebuginfodStats() + a.logGPUStats() if a.cpuProfiler != nil { a.cpuProfiler.Close() a.cpuProfiler = nil diff --git a/perfagent/options.go b/perfagent/options.go index ce2eb73..dc9e70f 100644 --- a/perfagent/options.go +++ b/perfagent/options.go @@ -68,6 +68,10 @@ type Config struct { // identical bytes at another path is a different target. GPUShimPath string + // GPUEventRingBytes sizes the BPF event ringbuf for the GPU pipeline. + // Zero keeps the compiled-in default. See WithGPUEventRingBytes. + GPUEventRingBytes int + // GPUProfilePath is the output path for the GPU profile. GPUProfilePath string @@ -373,6 +377,19 @@ func WithGPU(shimPath string) Option { } } +// WithGPUEventRingBytes sizes the BPF event ringbuf for the GPU pipeline. +// +// The default is sized for a capture at the shim's default sampling period. +// Lowering PERFAGENT_GPU_SAMPLE_PERIOD in the target multiplies the event +// rate, and the records that overflow are the kernel activity records that +// carry GPU time -- so an undersized buffer shrinks the profile rather than +// just coarsening it. Zero keeps the default. +func WithGPUEventRingBytes(n int) Option { + return func(c *Config) { + c.GPUEventRingBytes = n + } +} + // WithGPUProfilePath sets where the GPU profile is written. func WithGPUProfilePath(path string) Option { return func(c *Config) { c.GPUProfilePath = path }