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
