Re: [PATCHv4 02/14] uprobe: Add support for session consumer

Oleg Nesterov
2024-09-23T10:06:21Z

On 09/23, Jiri Olsa wrote:
>
> change below should do what you proposed originally

LGTM, just one nit below.

But I guess you need to do this on top of bpf/bpf.git, Andrii has already
applied your series.

And to remind, 02/14 must be fixed in any case unless I am totally confused,
handler_chain() can leak return_instance.

> also on top of that.. I discussed with Andrii the possibility of dropping
> the UPROBE_HANDLER_IWANTMYCOOKIE completely and setup cookie for any consumer
> that has both 'handler' and 'ret_handler' defined, wdyt?

Up to you. As I said from the very beginning I won't insist on _IWANTMYCOOKIE.

> list_for_each_entry_srcu(uc, &uprobe->consumers, cons_node,
> srcu_read_lock_held(&uprobes_srcu)) {
> + ric = return_consumer_find(ri, &ric_idx, uc->id);
> if (uc->ret_handler)
> - uc->ret_handler(uc, ri->func, regs);
> + uc->ret_handler(uc, ri->func, regs, ric ? &ric->cookie : NULL);
> }
> srcu_read_unlock(&uprobes_srcu, srcu_idx);

return_consumer_find() makes no sense if !uc->ret_handler, can you move

ric = return_consumer_find(ri, &ric_idx, uc->id);

into the "if (uc->ret_handler)" block?

Oleg.