Re: [PATCH v2 bpf-next 1/5] ftrace: allow customized flags for ftrace_direct_multi ftrace_ops

[Steven Rostedt]

On Thu, 14 Jul 2022 04:37:43 +0000
Song Liu <songliubraving@xxxxxx> wrote:

> >   
> >> 
> >> non-direct ops without IPMODIFY can already share with IPMODIFY ops.  
> > 
> > It can? ftrace sets IPMODIFY for all DIRECT callers to prevent that. Except
> > for this patch that removes that restriction (which I believe is broken).  
> 
> I mean "non-direct" ftrace ops, not direct ftrace ops. 

Ah, sorry misunderstood that.


> > Let me start from the beginning.  
> 
> I got your point now. We replace the flag on direct trampoline with a 
> callback check. So yes, this works. 

I'm glad we are on the same page :-)


> > 9. ftrace sees the lkp IPMODIFY ops has SHARED_IPMODIFY on it, and knows
> >   that there's a direct call here too. It removes the IPMODIFY ops, and
> >   then calls the direct ops->ops_func(STOP_SHARE_WITH_IPMODIFY) to let the
> >   direct code know that it is no longer sharing with an IPMODIFY such that
> >   it can change to call the function directly and not use the stack.  
> 
> I wonder whether we still need this flag. Alternatively, we can always
> find direct calls on the function and calls ops_func(STOP_SHARE_WITH_IPMODIFY). 

Actually we don't need the new flag and we don't need to always search. When
a direct is attached to the function then the rec->flags will have
FTRACE_FL_DIRECT attached to it.

Then if an IPMODIFY is being removed and the rec->flags has
FTRACE_FL_DIRECT set, then we know to search the ops for the one that has a
DIRECT flag attached and we can call the ops_func() on that one.

We should also add a FTRACE_WARN_ON() if a direct is not found but the flag
was set.

> 
> What do you think about this? 
>

I think this works.

Also, on the patch that implements this in the next version, please add to
the change log:

Link: https://lore.kernel.org/all/20220602193706.2607681-2-song@xxxxxxxxxx/

so that we have a link to this discussion.

Thanks,

-- Steve