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

[Oleg Nesterov <oleg@redhat.com>]

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.