Re: [PATCH v3 2/9] rv: add generic uprobe infrastructure for RV monitors

[Gabriele Monaco <gmonaco@redhat.com>]

On Wed, 2026-07-01 at 02:44 +0800, Wen Yang wrote:
> Thank you for the patient explanation.
> You are right, and v4 implements the embedded approach.
> struct rv_uprobe directly embeds struct uprobe_consumer:
> 
>    struct rv_uprobe {
>        struct uprobe_consumer  uc;
>        struct uprobe          *uprobe;
>        struct inode           *inode;
>        void                   *priv;
>        int (*handler)(...);
>        int (*ret_handler)(...);
>    };

Alright, great. Why are you still using double function pointers? Do you really
need rv_uprobe->handler instead of just using rv_uprobe->uc.handler ? That also
simplifies one function call down the road.

Thanks,
Gabriele

> rv_uprobe_free() is gone — no allocation means no explicit free.  After
> rv_uprobe_unregister() (or rv_uprobe_unregister_nosync() + 
> rv_uprobe_sync()), the caller frees the containing struct directly.  In 
> tlob:
> 
>    rv_uprobe_unregister_nosync(&b->start_probe);
>    rv_uprobe_unregister_nosync(&b->stop_probe);
>    rv_uprobe_sync();
>    kfree(b);   /* frees both embedded consumers */
> 
> This is the pattern from your sketch.
> 
> Regarding my earlier reply that argued for the separate allocation: I 
> was wrong. The key barrier is synchronize_rcu_tasks_trace() (called 
> first in uprobe_unregister_sync()), which waits for all 
> rcu_read_lock_trace() readers including handler_chain().  After it 
> returns, no cons_node.next read is in flight and embedding is safe.
> 
> We appreciate your thorough review. All of your comments have been 
> addressed in v4.
> We'll run local tests for one or two days, and then it will be sent out 
> shortly.
> 
> --
> Best wishes,
> Wen
>