* Re: [PATCH bpf-next 09/10] tools: bpftool: add simple perf event output reader
[not found] ` <20180504212501.hn2rnv7t3ik563mg@ast-mbp>
@ 2018-05-04 22:28 ` Jakub Kicinski
0 siblings, 0 replies; only message in thread
From: Jakub Kicinski @ 2018-05-04 22:28 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: daniel, oss-drivers, netdev, linux-kernel, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
Jiri Olsa, Namhyung Kim
CC perf folks
On Fri, 4 May 2018 14:25:03 -0700, Alexei Starovoitov wrote:
> > +static void
> > +perf_event_read(struct event_ring_info *ring, void **buf, size_t *buf_len)
> > +{
> > + volatile struct perf_event_mmap_page *header = ring->mem;
> > + __u64 buffer_size = MMAP_PAGE_CNT * get_page_size();
> > + __u64 data_tail = header->data_tail;
> > + __u64 data_head = header->data_head;
> > + void *base, *begin, *end;
> > +
> > + asm volatile("" ::: "memory"); /* in real code it should be smp_rmb() */
> > + if (data_head == data_tail)
> > + return;
>
> this function was copied several times into different places.
> I think it's time to put into common lib. Like libbpf.
Agreed, I think libbpf would work, although there is nothing BPF
specific in this loop AFAICT now.
> Would be great if you can do it in the follow up.
Looking into it now, I found these:
$ git grep 'data_head == data_tail'
tools/bpf/bpftool/map_perf_ring.c: if (data_head == data_tail)
tools/testing/selftests/bpf/trace_helpers.c: if (data_head == data_tail)
Are there any other copies I should try to cater to? I have change a few
things compared to the selftest, I guess others may have modified their
copy too. Just trying to make sure what we put in libbpf would cater
to most possible use cases.
Should I also move bpf_perf_event_open()/test_bpf_perf_event() to libbpf?
> for the set:
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Thanks!
^ permalink raw reply [flat|nested] only message in thread