Re: [PATCH 1/3] ftrace: Fix updating of filters for shared global_ops filters
(2015/01/15 0:39), Steven Rostedt wrote: > From: "Steven Rostedt (Red Hat)" > > As the set_ftrace_filter affects both the function tracer as well as the > function graph tracer, the ops that represent each have a shared > ftrace_ops_hash structure. This allows both to be updated when the filter > files are updated. > > But if function graph is enabled and the global_ops (function tracing) ops > is not, then it is possible that the filter could be changed without the > update happening for the function graph ops. This will cause the changes > to not take place and may even cause a ftrace_bug to occur as it could mess > with the trampoline accounting. > > The solution is to check if the ops uses the shared global_ops filter and > if the ops itself is not enabled, to check if there's another ops that is > enabled and also shares the global_ops filter. In that case, the > modification still needs to be executed. > Looks good to me. Reviewed-by: Masami Hiramatsu Thank you, > Cc: sta...@vger.kernel.org # 3.17+ > Signed-off-by: Steven Rostedt > --- > kernel/trace/ftrace.c | 26 +- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 929a733d302e..2b35d0ba578d 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -4008,8 +4008,32 @@ ftrace_match_addr(struct ftrace_hash *hash, unsigned > long ip, int remove) > static void ftrace_ops_update_code(struct ftrace_ops *ops, > struct ftrace_hash *old_hash) > { > - if (ops->flags & FTRACE_OPS_FL_ENABLED && ftrace_enabled) > + struct ftrace_ops *op; > + > + if (!ftrace_enabled) > + return; > + > + if (ops->flags & FTRACE_OPS_FL_ENABLED) { > ftrace_run_modify_code(ops, FTRACE_UPDATE_CALLS, old_hash); > + return; > + } > + > + /* > + * If this is the shared global_ops filter, then we need to > + * check if there is another ops that shares it, is enabled. > + * If so, we still need to run the modify code. > + */ > + if (ops->func_hash != _ops.local_hash) > + return; > + > + do_for_each_ftrace_op(op, ftrace_ops_list) { > + if (op->func_hash == _ops.local_hash && > + op->flags & FTRACE_OPS_FL_ENABLED) { > + ftrace_run_modify_code(op, FTRACE_UPDATE_CALLS, > old_hash); > + /* Only need to do this once */ > + return; > + } > + } while_for_each_ftrace_op(op); > } > > static int -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] ftrace: Fix updating of filters for shared global_ops filters
(2015/01/15 0:39), Steven Rostedt wrote: From: Steven Rostedt (Red Hat) rost...@goodmis.org As the set_ftrace_filter affects both the function tracer as well as the function graph tracer, the ops that represent each have a shared ftrace_ops_hash structure. This allows both to be updated when the filter files are updated. But if function graph is enabled and the global_ops (function tracing) ops is not, then it is possible that the filter could be changed without the update happening for the function graph ops. This will cause the changes to not take place and may even cause a ftrace_bug to occur as it could mess with the trampoline accounting. The solution is to check if the ops uses the shared global_ops filter and if the ops itself is not enabled, to check if there's another ops that is enabled and also shares the global_ops filter. In that case, the modification still needs to be executed. Looks good to me. Reviewed-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com Thank you, Cc: sta...@vger.kernel.org # 3.17+ Signed-off-by: Steven Rostedt rost...@goodmis.org --- kernel/trace/ftrace.c | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 929a733d302e..2b35d0ba578d 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -4008,8 +4008,32 @@ ftrace_match_addr(struct ftrace_hash *hash, unsigned long ip, int remove) static void ftrace_ops_update_code(struct ftrace_ops *ops, struct ftrace_hash *old_hash) { - if (ops-flags FTRACE_OPS_FL_ENABLED ftrace_enabled) + struct ftrace_ops *op; + + if (!ftrace_enabled) + return; + + if (ops-flags FTRACE_OPS_FL_ENABLED) { ftrace_run_modify_code(ops, FTRACE_UPDATE_CALLS, old_hash); + return; + } + + /* + * If this is the shared global_ops filter, then we need to + * check if there is another ops that shares it, is enabled. + * If so, we still need to run the modify code. + */ + if (ops-func_hash != global_ops.local_hash) + return; + + do_for_each_ftrace_op(op, ftrace_ops_list) { + if (op-func_hash == global_ops.local_hash + op-flags FTRACE_OPS_FL_ENABLED) { + ftrace_run_modify_code(op, FTRACE_UPDATE_CALLS, old_hash); + /* Only need to do this once */ + return; + } + } while_for_each_ftrace_op(op); } static int -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] ftrace: Fix updating of filters for shared global_ops filters
From: "Steven Rostedt (Red Hat)" As the set_ftrace_filter affects both the function tracer as well as the function graph tracer, the ops that represent each have a shared ftrace_ops_hash structure. This allows both to be updated when the filter files are updated. But if function graph is enabled and the global_ops (function tracing) ops is not, then it is possible that the filter could be changed without the update happening for the function graph ops. This will cause the changes to not take place and may even cause a ftrace_bug to occur as it could mess with the trampoline accounting. The solution is to check if the ops uses the shared global_ops filter and if the ops itself is not enabled, to check if there's another ops that is enabled and also shares the global_ops filter. In that case, the modification still needs to be executed. Cc: sta...@vger.kernel.org # 3.17+ Signed-off-by: Steven Rostedt --- kernel/trace/ftrace.c | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 929a733d302e..2b35d0ba578d 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -4008,8 +4008,32 @@ ftrace_match_addr(struct ftrace_hash *hash, unsigned long ip, int remove) static void ftrace_ops_update_code(struct ftrace_ops *ops, struct ftrace_hash *old_hash) { - if (ops->flags & FTRACE_OPS_FL_ENABLED && ftrace_enabled) + struct ftrace_ops *op; + + if (!ftrace_enabled) + return; + + if (ops->flags & FTRACE_OPS_FL_ENABLED) { ftrace_run_modify_code(ops, FTRACE_UPDATE_CALLS, old_hash); + return; + } + + /* +* If this is the shared global_ops filter, then we need to +* check if there is another ops that shares it, is enabled. +* If so, we still need to run the modify code. +*/ + if (ops->func_hash != _ops.local_hash) + return; + + do_for_each_ftrace_op(op, ftrace_ops_list) { + if (op->func_hash == _ops.local_hash && + op->flags & FTRACE_OPS_FL_ENABLED) { + ftrace_run_modify_code(op, FTRACE_UPDATE_CALLS, old_hash); + /* Only need to do this once */ + return; + } + } while_for_each_ftrace_op(op); } static int -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] ftrace: Fix updating of filters for shared global_ops filters
From: Steven Rostedt (Red Hat) rost...@goodmis.org As the set_ftrace_filter affects both the function tracer as well as the function graph tracer, the ops that represent each have a shared ftrace_ops_hash structure. This allows both to be updated when the filter files are updated. But if function graph is enabled and the global_ops (function tracing) ops is not, then it is possible that the filter could be changed without the update happening for the function graph ops. This will cause the changes to not take place and may even cause a ftrace_bug to occur as it could mess with the trampoline accounting. The solution is to check if the ops uses the shared global_ops filter and if the ops itself is not enabled, to check if there's another ops that is enabled and also shares the global_ops filter. In that case, the modification still needs to be executed. Cc: sta...@vger.kernel.org # 3.17+ Signed-off-by: Steven Rostedt rost...@goodmis.org --- kernel/trace/ftrace.c | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 929a733d302e..2b35d0ba578d 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -4008,8 +4008,32 @@ ftrace_match_addr(struct ftrace_hash *hash, unsigned long ip, int remove) static void ftrace_ops_update_code(struct ftrace_ops *ops, struct ftrace_hash *old_hash) { - if (ops-flags FTRACE_OPS_FL_ENABLED ftrace_enabled) + struct ftrace_ops *op; + + if (!ftrace_enabled) + return; + + if (ops-flags FTRACE_OPS_FL_ENABLED) { ftrace_run_modify_code(ops, FTRACE_UPDATE_CALLS, old_hash); + return; + } + + /* +* If this is the shared global_ops filter, then we need to +* check if there is another ops that shares it, is enabled. +* If so, we still need to run the modify code. +*/ + if (ops-func_hash != global_ops.local_hash) + return; + + do_for_each_ftrace_op(op, ftrace_ops_list) { + if (op-func_hash == global_ops.local_hash + op-flags FTRACE_OPS_FL_ENABLED) { + ftrace_run_modify_code(op, FTRACE_UPDATE_CALLS, old_hash); + /* Only need to do this once */ + return; + } + } while_for_each_ftrace_op(op); } static int -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/