On Fri, Aug 23, 2024 at 11:49 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Tue, Aug 13, 2024 at 2:24 PM Amery Hung <amery.hung@xxxxxxxxxxxxx> wrote:
> >
> > From: Dave Marchevsky <davemarchevsky@xxxxxx>
> >
> > Test stashing both referenced kptr and local kptr into local kptrs. Then,
> > test unstashing them.
> >
> > Acked-by: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
> > Acked-by: Hou Tao <houtao1@xxxxxxxxxx>
> > Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
> > Signed-off-by: Amery Hung <amery.hung@xxxxxxxxxxxxx>
> > ---
> > .../selftests/bpf/progs/local_kptr_stash.c | 30 +++++++++++++++++--
> > .../selftests/bpf/progs/task_kfunc_success.c | 26 +++++++++++++++-
> > 2 files changed, 53 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/local_kptr_stash.c b/tools/testing/selftests/bpf/progs/local_kptr_stash.c
> > index 75043ffc5dad..b092a72b2c9d 100644
> > --- a/tools/testing/selftests/bpf/progs/local_kptr_stash.c
> > +++ b/tools/testing/selftests/bpf/progs/local_kptr_stash.c
> > @@ -8,9 +8,12 @@
> > #include "../bpf_experimental.h"
> > #include "../bpf_testmod/bpf_testmod_kfunc.h"
> >
> > +struct plain_local;
> > +
> > struct node_data {
> > long key;
> > long data;
> > + struct plain_local __kptr * stashed_in_local_kptr;
> > struct bpf_rb_node node;
> > };
>
> Everything looks correct and I applied the set.
> The selftest sort-of covers the case where stashed_in_local_kptr
> is being freed by rb_root recursive freeing,
> but it doesn't really check for memory leaks.
> It only checks that nothing will crash.
>
> Please follow up with an improvement to selftest that
> actually makes sure that recursive freeing of stashed kptr
> correctly calls bpf_obj_free_fields->__bpf_obj_drop_impl.
>
Will do. Thanks for reviewing the patchset!
> The patches seem to do the right thing in terms of storing
> correct btf/records in the right places,
> but this is tricky, so extra tests are warranted.
