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.


Reply via email to