Re: [PATCH ftrace/core v6 2/5] ftrace, kprobes: Support IPMODIFY flag to find IP modify conflict

Steven Rostedt
2014-11-21T19:43:28Z

On Fri, 21 Nov 2014 13:05:29 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 21 Nov 2014 05:25:16 -0500
> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

> > + * IPMODIFY - The ops can modify the IP register. This can only be set with
> > + * SAVE_REGS. If another ops is already registered for any of the
> > + * functions that this ops will be registered for, then this ops
> > + * will fail to register or set_filter_ip.
>
> It's blocked by any ops sharing the same function, or just another ops
> with this flag set? The comment doesn't specify. The code looks like
> the latter.

I applied it and gave it the following updates to comments:

-- Steve

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 93cf0478f64e..ed501953f0b2 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -107,9 +107,9 @@ ftrace_func_t ftrace_ops_get_func(struct ftrace_ops *ops);
* in such cases the arch must not modify it. Only the arch ftrace
* core code should set this flag.
* IPMODIFY - The ops can modify the IP register. This can only be set with
- * SAVE_REGS. If another ops is already registered for any of the
- * functions that this ops will be registered for, then this ops
- * will fail to register or set_filter_ip.
+ * SAVE_REGS. If another ops with this flag set is already registered
+ * for any of the functions that this ops will be registered for, then
+ * this ops will fail to register or set_filter_ip.
*/
enum {
FTRACE_OPS_FL_ENABLED = 1 << 0,
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 531e72a716c3..929a733d302e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1810,8 +1810,8 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
return 0;

/*
- * Since the IPMODIFY is very address sensitive action, we do not allow
- * ftrace_ops to set all functions to new hash.
+ * Since the IPMODIFY is a very address sensitive action, we do not
+ * allow ftrace_ops to set all functions to new hash.
*/
if (!new_hash || !old_hash)
return -EINVAL;