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 <paul...@kernel.org>
> Cc: Jonathan Lemon <jonathan.le...@gmail.com>

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

Reply via email to