Re: [PATCH bpf-next 0/6] BPF ring buffer
From: Jonathan Lemon <hidden>
Date: 2020-05-13 22:56:14
Also in:
bpf
On Wed, May 13, 2020 at 12:25:26PM -0700, Andrii Nakryiko wrote:
Implement a new BPF ring buffer, as presented at BPF virtual conference ([0]). It presents an alternative to perf buffer, following its semantics closely, but allowing sharing same instance of ring buffer across multiple CPUs efficiently. Most patches have extensive commentary explaining various aspects, so I'll keep cover letter short. Overall structure of the patch set: - patch #1 adds BPF ring buffer implementation to kernel and necessary verifier support; - patch #2 adds litmus tests validating all the memory orderings and locking is correct; - patch #3 is an optional patch that generalizes verifier's reference tracking machinery to capture type of reference; - patch #4 adds libbpf consumer implementation for BPF ringbuf; - path #5 adds selftest, both for single BPF ring buf use case, as well as using it with array/hash of maps; - patch #6 adds extensive benchmarks and provide some analysis in commit message, it build upon selftests/bpf's bench runner. [0] https://docs.google.com/presentation/d/18ITdg77Bj6YDOH2LghxrnFxiPWe0fAqcmJY95t_qr0w Cc: Paul E. McKenney <paulmck@kernel.org> Cc: Jonathan Lemon <redacted>
Looks very nice! A few random questions:
1) Why not use a structure for the header, instead of 2 32bit ints?
2) Would it make sense to reserve X bytes, but only commit Y?
the offset field could be used to write the record length.
E.g.:
reserve 512 bytes [BUSYBIT | 512][PG OFFSET]
commit 400 bytes [ 512 ] [ 400 ]
3) Why have 2 separate pages for producer/consumer, instead of
just aligning to a smp cache line (or even 1/2 page?)
4) The XOR of busybit makes me wonder if there is anything that
prevents the system from calling commit twice?
--
Jonathan