Messages in this thread | | | Date | Fri, 21 Feb 2014 13:01:02 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH 0/4] x86: Fix ftrace recovery when code modification failed |
| |
On Fri, 21 Feb 2014 17:33:52 +0100 Petr Mládek <pmladek@suse.cz> wrote:
> Yes, it works but I would still do few more changes: > > + run_sync() also in ftrace_modify_code() in the recovery > part; I would solve this by moving the "out" label.
Sounds good.
> > + print some warning in update_ftrace_func() when > ftrace_modify_code() fails; otherwise, the error > is silently ignored
The error is passed down to the callers. Any warning should happen there. We can move that up to the generic code. Otherwise, this is a x86 only fix, and we leave all the other archs broken.
ftrace_modify_all_code() looks to be where the warnings should be. And yes, any warning should also call ftrace_bug(). That can be done via a FTRACE_WARN_ON().
> > + BUG when the breakpoint cannot be removed; the system > silently crash anyway because the breakpoint will not > be handled > > I wonder if we should call FTRACE_WARN_ON_ONCE that calls > "ftrace_kill" when update_ftrace_func() fails.
Don't need the _ONCE() version. Something like this (I'll let you make the patch ;-)
int ret = 0;
if (update) ret = ftrace_update_ftrace_func(ftrace_ops_list_func); if (ret) goto out;
if (command & FTRACE_UPDATE_CALLS) ret = ftrace_replace_code(1); else if (command & FTRACE_DISABLE_CALLS) ret = ftrace_replace_code(0);
if (ret) goto out;
if (update && ftrace_trace_function != ftrace_ops_list_func) { function_trace_op = set_function_trace_op; smp_wmb(); /* If irqs are disabled, we are in stop machine */ if (!irqs_disabled()) smp_call_function(ftrace_sync_ipi, NULL, 1); ret = ftrace_update_ftrace_func(ftrace_trace_function); }
if (ret) goto out;
if (command & FTRACE_START_FUNC_RET) ret = ftrace_enable_ftrace_graph_caller(); else if (command & FTRACE_STOP_FUNC_RET) ret = ftrace_disable_ftrace_graph_caller();
out: FTRACE_WARN_ON(ret);
Or if you want to keep where it failed, just replace the if (ret) goto out, with:
if (FTRACE_WARN_ON(ret)) return;
That may actually be better.
I've already added my patch. I'll push it up to my ftrace/urgent branch at
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
And you can write a patch based on that.
Thanks!
-- Steve
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |