On Wed, 2024-06-19 at 17:45 +0100, Alan Maguire wrote:
[...]
> oops, missed a GFP_ATOMIC here to avoid possible sleeping. To use
> existing kfunc call test structure it's simpler to do this than add a
> sleepable test context I think, especially since the focus here is on
> adding a basic test. More below..
Hi Alan,
And I agree, GFP_ATOMIC should probably help and it is simpler than
adding a sleepable context.
[...]
> Yeah, my focus here was testing the registration to be honest and
> thankfully as you noted it caught a case where I had forgotten to do id
> relocation, so thanks for suggesting this!
>
> To trigger the dtor cleanup via a map, I came up with the following:
>
> - call bpf_testmod_ctx_create()
> - do bpf_kptr_xchg(&ctx_val->ctx, ctx) to transfer the ctx kptr into the
> map value;
> - only release the reference if the kptr exchange fails
> - and then it gets cleaned up on exit.
>
> I haven't used kptrs much so hopefully that's right.
>
> Tracing I confirmed cleanup happens via:
>
> $ sudo dtrace -n 'fbt::bpf_testmod_ctx_release:entry { stack(); }'
> dtrace: description 'fbt::bpf_testmod_ctx_release:entry ' matched 1 probe
> CPU ID FUNCTION:NAME
> 3 113779 bpf_testmod_ctx_release:entry
> vmlinux`array_map_free+0x69
> vmlinux`bpf_map_free_deferred+0x62
> vmlinux`process_one_work+0x192
> vmlinux`worker_thread+0x27a
> vmlinux`kthread+0xf7
> vmlinux`ret_from_fork+0x41
> vmlinux`ret_from_fork_asm+0x1a
>
> Does the above sound right? Thanks!
It does, might as well set some flag in the dtor kfunc and check it in
the program (using another map?). Tbh, I thought we could get away w/o
complicating the test, but since you already have it working, we should
probably add it.
Thanks,
Eduard
