| Message ID | 20210328161055.257504-3-pctammela@mojatatu.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] libbpf: Add '_wait()' and '_nowait()' macros for 'bpf_ring_buffer__poll()' | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 14 of 14 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: From:/Signed-off-by: email address mismatch: 'From: Pedro Tammela <pctammela@gmail.com>' != 'Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>' |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
> On Mar 28, 2021, at 9:10 AM, Pedro Tammela <pctammela@gmail.com> wrote: > > 'bpf_ring_buffer__poll()' abstracts the polling method, so abstract the > constants that make the implementation don't wait or wait indefinetly > for data. > > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > --- > tools/lib/bpf/libbpf.h | 3 +++ > tools/testing/selftests/bpf/benchs/bench_ringbufs.c | 2 +- > tools/testing/selftests/bpf/prog_tests/ringbuf.c | 6 +++--- > tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c | 4 ++-- > 4 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > index f500621d28e5..3817d84f91c6 100644 > --- a/tools/lib/bpf/libbpf.h > +++ b/tools/lib/bpf/libbpf.h > @@ -540,6 +540,9 @@ LIBBPF_API int ring_buffer__poll(struct ring_buffer *rb, int timeout_ms); > LIBBPF_API int ring_buffer__consume(struct ring_buffer *rb); > LIBBPF_API int ring_buffer__epoll_fd(const struct ring_buffer *rb); > > +#define ring_buffer__poll_wait(rb) ring_buffer__poll(rb, -1) > +#define ring_buffer__poll_nowait(rb) ring_buffer__poll(rb, 0) I think we don't need ring_buffer__poll_wait() as ring_buffer__poll() already means "wait for timeout_ms". Actually, I think ring_buffer__poll() is enough. ring_buffer__poll_nowait() is not that useful either. Thanks, Song
On Mon, Mar 29, 2021 at 9:28 AM Song Liu <songliubraving@fb.com> wrote: > > > > > On Mar 28, 2021, at 9:10 AM, Pedro Tammela <pctammela@gmail.com> wrote: > > > > 'bpf_ring_buffer__poll()' abstracts the polling method, so abstract the > > constants that make the implementation don't wait or wait indefinetly > > for data. > > > > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > > --- > > tools/lib/bpf/libbpf.h | 3 +++ > > tools/testing/selftests/bpf/benchs/bench_ringbufs.c | 2 +- > > tools/testing/selftests/bpf/prog_tests/ringbuf.c | 6 +++--- > > tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c | 4 ++-- > > 4 files changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > > index f500621d28e5..3817d84f91c6 100644 > > --- a/tools/lib/bpf/libbpf.h > > +++ b/tools/lib/bpf/libbpf.h > > @@ -540,6 +540,9 @@ LIBBPF_API int ring_buffer__poll(struct ring_buffer *rb, int timeout_ms); > > LIBBPF_API int ring_buffer__consume(struct ring_buffer *rb); > > LIBBPF_API int ring_buffer__epoll_fd(const struct ring_buffer *rb); > > > > +#define ring_buffer__poll_wait(rb) ring_buffer__poll(rb, -1) > > +#define ring_buffer__poll_nowait(rb) ring_buffer__poll(rb, 0) > > I think we don't need ring_buffer__poll_wait() as ring_buffer__poll() already > means "wait for timeout_ms". > > Actually, I think ring_buffer__poll() is enough. ring_buffer__poll_nowait() > is not that useful either. > I agree. I think adding a comment to the API itself might be useful specifying 0 and -1 as somewhat special cases. > Thanks, > Song >
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index f500621d28e5..3817d84f91c6 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -540,6 +540,9 @@ LIBBPF_API int ring_buffer__poll(struct ring_buffer *rb, int timeout_ms); LIBBPF_API int ring_buffer__consume(struct ring_buffer *rb); LIBBPF_API int ring_buffer__epoll_fd(const struct ring_buffer *rb); +#define ring_buffer__poll_wait(rb) ring_buffer__poll(rb, -1) +#define ring_buffer__poll_nowait(rb) ring_buffer__poll(rb, 0) + /* Perf buffer APIs */ struct perf_buffer; diff --git a/tools/testing/selftests/bpf/benchs/bench_ringbufs.c b/tools/testing/selftests/bpf/benchs/bench_ringbufs.c index bde6c9d4cbd4..82db2cc9bab3 100644 --- a/tools/testing/selftests/bpf/benchs/bench_ringbufs.c +++ b/tools/testing/selftests/bpf/benchs/bench_ringbufs.c @@ -191,7 +191,7 @@ static void *ringbuf_libbpf_consumer(void *input) { struct ringbuf_libbpf_ctx *ctx = &ringbuf_libbpf_ctx; - while (ring_buffer__poll(ctx->ringbuf, -1) >= 0) { + while (ring_buffer__poll_wait(ctx->ringbuf) >= 0) { if (args.back2back) bufs_trigger_batch(); } diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf.c b/tools/testing/selftests/bpf/prog_tests/ringbuf.c index fddbc5db5d6a..321c646a0685 100644 --- a/tools/testing/selftests/bpf/prog_tests/ringbuf.c +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf.c @@ -121,7 +121,7 @@ void test_ringbuf(void) 3L * rec_sz, skel->bss->prod_pos); /* poll for samples */ - err = ring_buffer__poll(ringbuf, -1); + err = ring_buffer__poll_wait(ringbuf); /* -EDONE is used as an indicator that we are done */ if (CHECK(err != -EDONE, "err_done", "done err: %d\n", err)) @@ -130,7 +130,7 @@ void test_ringbuf(void) CHECK(cnt != 2, "cnt", "exp %d samples, got %d\n", 2, cnt); /* we expect extra polling to return nothing */ - err = ring_buffer__poll(ringbuf, 0); + err = ring_buffer__poll_nowait(ringbuf); if (CHECK(err != 0, "extra_samples", "poll result: %d\n", err)) goto cleanup; cnt = atomic_xchg(&sample_cnt, 0); @@ -148,7 +148,7 @@ void test_ringbuf(void) CHECK(skel->bss->cons_pos != 3 * rec_sz, "err_cons_pos", "exp %ld, got %ld\n", 3L * rec_sz, skel->bss->cons_pos); - err = ring_buffer__poll(ringbuf, -1); + err = ring_buffer__poll_wait(ringbuf); CHECK(err <= 0, "poll_err", "err %d\n", err); cnt = atomic_xchg(&sample_cnt, 0); CHECK(cnt != 2, "cnt", "exp %d samples, got %d\n", 2, cnt); diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c index d37161e59bb2..65ba0a3472f1 100644 --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c @@ -80,12 +80,12 @@ void test_ringbuf_multi(void) syscall(__NR_getpgid); /* poll for samples, should get 2 ringbufs back */ - err = ring_buffer__poll(ringbuf, -1); + err = ring_buffer__poll_wait(ringbuf); if (CHECK(err != 2, "poll_res", "expected 2 records, got %d\n", err)) goto cleanup; /* expect extra polling to return nothing */ - err = ring_buffer__poll(ringbuf, 0); + err = ring_buffer__poll_nowait(ringbuf); if (CHECK(err < 0, "extra_samples", "poll result: %d\n", err)) goto cleanup;
'bpf_ring_buffer__poll()' abstracts the polling method, so abstract the constants that make the implementation don't wait or wait indefinetly for data. Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> --- tools/lib/bpf/libbpf.h | 3 +++ tools/testing/selftests/bpf/benchs/bench_ringbufs.c | 2 +- tools/testing/selftests/bpf/prog_tests/ringbuf.c | 6 +++--- tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-)