Re: [PATCH v2 bpf-next 1/5] ftrace: allow customized flags for ftrace_direct_multi ftrace_ops
From: Steven Rostedt
Date: Thu Jul 14 2022 - 09:23:31 EST
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