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

Oleg Nesterov
2024-09-22T15:27:50Z

Damn, sorry for delay :/

And sorry, still can't understand, see below...

On 09/17, Jiri Olsa wrote:
>
> On Tue, Sep 17, 2024 at 02:03:17PM +0200, Oleg Nesterov wrote:
> >
> > To me this code should do:
> >
> > if (!uc->ret_handler || UPROBE_HANDLER_REMOVE || UPROBE_HANDLER_IGNORE)
> > continue;
> >
> > if (!ri)
> > ri = alloc_return_instance();
> >
> > if (rc == UPROBE_HANDLER_IWANTMYCOOKIE)
> > ri = push_consumer(...);
> >
> > And,
> >
> > > handle_uretprobe_chain(struct return_instance *ri, struct pt_regs *regs)
> > ...
> > > 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 (ric && ric->rc == UPROBE_HANDLER_IGNORE)
> > > + continue;
> > > if (uc->ret_handler)
> > > - uc->ret_handler(uc, ri->func, regs);
> > > + uc->ret_handler(uc, ri->func, regs, ric ? &ric->cookie : NULL);
> > > }
> >
> > the UPROBE_HANDLER_IGNORE check above and the new ric->rc member should die,
> >
> > if (!uc->ret_handler)
> > continue;
> >
> > ric = return_consumer_find(...);
> > uc->ret_handler(..., ric ? &ric->cookie : NULL);
> >
> > as we have already discussed, the session ret_handler(data) can simply do
> >
> > // my ->handler() wasn't called or it didn't return
> > // UPROBE_HANDLER_IWANTMYCOOKIE
> > if (!data)
> > return;
> >
> > at the start.
> >
> > Could you explain why this can't work?
>
> I'll try ;-) it's for the case when consumer does not use UPROBE_HANDLER_IWANTMYCOOKIE
>
> let's have 2 consumers on single uprobe, consumer-A returning UPROBE_HANDLER_IGNORE
> and the consumer-B returning zero, so we want the return uprobe installed, but we
> want just consumer-B to be executed
>
> - so uprobe gets installed and handle_uretprobe_chain goes over all consumers
> calling ret_handler callback
>
> - but we don't know consumer-A needs to be ignored, and it does not
> expect cookie so we have no way to find out it needs to be ignored

How does this differ from the case when consumer-A returns _REMOVE but another
consumer returns 0?

But what I really can't understand is

and it does not
expect cookie so we have no way to find out it needs to be ignored

If we change the code as I suggested above, push_consumer() won't be called
if consumer-A returns UPROBE_HANDLER_IGNORE.

This means that handle_uretprobe_chain() -> return_consumer_find() will
return NULL, so handle_uretprobe_chain() won't pass the valid cookie to
consumer-A's ret_handler callback, it will pass data => NULL.

So, again, why can't consumer-A's ret_handler callback do

// my ->handler() wasn't called or it didn't return
// UPROBE_HANDLER_IWANTMYCOOKIE
if (!data)
return;

at the start?

Why the UPROBE_HANDLER_IGNORE case is more problematic than the
UPROBE_HANDLER_REMOVE case?

Oleg.