| Message ID | 20240705-fix-wq-v1-2-91b4d82cd825@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Small API fix for bpf_wq | expand |
On Fri, 2024-07-05 at 15:44 +0200, Benjamin Tissoires wrote: > See the previous patch: the API was wrong, we were provided the pointer > to the value, not the actual struct bpf_wq *. > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> > --- Would it make sense to update one of the tests, so that it checks the specific value put in the map? E.g. extend struct elem: struct elem { int answer_to_the_ultimate_question; struct bpf_wq w; }; And put something in there? [...]
On Fri, Jul 5, 2024 at 1:54 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > On Fri, 2024-07-05 at 15:44 +0200, Benjamin Tissoires wrote: > > See the previous patch: the API was wrong, we were provided the pointer > > to the value, not the actual struct bpf_wq *. > > > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> > > --- > > Would it make sense to update one of the tests, so that it checks the > specific value put in the map? > E.g. extend struct elem: > > struct elem { > int answer_to_the_ultimate_question; > struct bpf_wq w; > }; > > And put something in there? +1 let's tweak the selftest. pw-bot: cr
On Jul 06 2024, Alexei Starovoitov wrote: > On Fri, Jul 5, 2024 at 1:54 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > > > On Fri, 2024-07-05 at 15:44 +0200, Benjamin Tissoires wrote: > > > See the previous patch: the API was wrong, we were provided the pointer > > > to the value, not the actual struct bpf_wq *. > > > > > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> > > > --- > > > > Would it make sense to update one of the tests, so that it checks the > > specific value put in the map? > > E.g. extend struct elem: > > > > struct elem { > > int answer_to_the_ultimate_question; > > struct bpf_wq w; > > }; > > > > And put something in there? > > +1 > let's tweak the selftest. OK, v2 it is then :) Cheers, Benjamin
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h index eede6fc2ccb4..828556cdc2f0 100644 --- a/tools/testing/selftests/bpf/bpf_experimental.h +++ b/tools/testing/selftests/bpf/bpf_experimental.h @@ -552,7 +552,7 @@ extern void bpf_iter_css_destroy(struct bpf_iter_css *it) __weak __ksym; extern int bpf_wq_init(struct bpf_wq *wq, void *p__map, unsigned int flags) __weak __ksym; extern int bpf_wq_start(struct bpf_wq *wq, unsigned int flags) __weak __ksym; extern int bpf_wq_set_callback_impl(struct bpf_wq *wq, - int (callback_fn)(void *map, int *key, struct bpf_wq *wq), + int (callback_fn)(void *map, int *key, void *value), unsigned int flags__k, void *aux__ign) __ksym; #define bpf_wq_set_callback(timer, cb, flags) \ bpf_wq_set_callback_impl(timer, cb, flags, NULL) diff --git a/tools/testing/selftests/bpf/progs/wq.c b/tools/testing/selftests/bpf/progs/wq.c index 49e712acbf60..e5ac0df59b86 100644 --- a/tools/testing/selftests/bpf/progs/wq.c +++ b/tools/testing/selftests/bpf/progs/wq.c @@ -53,7 +53,7 @@ __u32 ok; __u32 ok_sleepable; static int test_elem_callback(void *map, int *key, - int (callback_fn)(void *map, int *key, struct bpf_wq *wq)) + int (callback_fn)(void *map, int *key, void *value)) { struct elem init = {}, *val; struct bpf_wq *wq; @@ -84,7 +84,7 @@ static int test_elem_callback(void *map, int *key, } static int test_hmap_elem_callback(void *map, int *key, - int (callback_fn)(void *map, int *key, struct bpf_wq *wq)) + int (callback_fn)(void *map, int *key, void *value)) { struct hmap_elem init = {}, *val; struct bpf_wq *wq; @@ -114,7 +114,7 @@ static int test_hmap_elem_callback(void *map, int *key, } /* callback for non sleepable workqueue */ -static int wq_callback(void *map, int *key, struct bpf_wq *work) +static int wq_callback(void *map, int *key, void *value) { bpf_kfunc_common_test(); ok |= (1 << *key); @@ -122,7 +122,7 @@ static int wq_callback(void *map, int *key, struct bpf_wq *work) } /* callback for sleepable workqueue */ -static int wq_cb_sleepable(void *map, int *key, struct bpf_wq *work) +static int wq_cb_sleepable(void *map, int *key, void *value) { bpf_kfunc_call_test_sleepable(); ok_sleepable |= (1 << *key); diff --git a/tools/testing/selftests/bpf/progs/wq_failures.c b/tools/testing/selftests/bpf/progs/wq_failures.c index 4cbdb425f223..25b51a72fe0f 100644 --- a/tools/testing/selftests/bpf/progs/wq_failures.c +++ b/tools/testing/selftests/bpf/progs/wq_failures.c @@ -28,14 +28,14 @@ struct { } lru SEC(".maps"); /* callback for non sleepable workqueue */ -static int wq_callback(void *map, int *key, struct bpf_wq *work) +static int wq_callback(void *map, int *key, void *value) { bpf_kfunc_common_test(); return 0; } /* callback for sleepable workqueue */ -static int wq_cb_sleepable(void *map, int *key, struct bpf_wq *work) +static int wq_cb_sleepable(void *map, int *key, void *value) { bpf_kfunc_call_test_sleepable(); return 0;
See the previous patch: the API was wrong, we were provided the pointer to the value, not the actual struct bpf_wq *. Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> --- tools/testing/selftests/bpf/bpf_experimental.h | 2 +- tools/testing/selftests/bpf/progs/wq.c | 8 ++++---- tools/testing/selftests/bpf/progs/wq_failures.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)