On Mon 2019-10-14 11:17:19, Steven Rostedt wrote: > On Mon, 14 Oct 2019 12:59:23 +0200 > Miroslav Benes <[email protected]> wrote: > > > int > > ftrace_enable_sysctl(struct ctl_table *table, int write, > > void __user *buffer, size_t *lenp, > > @@ -6740,8 +6754,6 @@ ftrace_enable_sysctl(struct ctl_table *table, int > > write, > > if (ret || !write || (last_ftrace_enabled == !!ftrace_enabled)) > > goto out; > > > > - last_ftrace_enabled = !!ftrace_enabled; > > - > > if (ftrace_enabled) { > > > > /* we are starting ftrace again */ > > @@ -6752,12 +6764,19 @@ ftrace_enable_sysctl(struct ctl_table *table, int > > write, > > ftrace_startup_sysctl(); > > > > } else { > > + if (is_permanent_ops_registered()) { > > + ftrace_enabled = last_ftrace_enabled; > > Although this is not incorrect, but may be somewhat confusing. > > At this location, last_ftrace_enabled is always true. > > I'm thinking this would be better to simply set it to false here.
IMHO, we want to set ftrace_enabled = true here. It was set to "false" by writing to the sysfs file. But the change gets rejected. ftrace will stay enabled. So, we should set the value back to "true". > > + ret = -EBUSY; > > + goto out; > > + } > > + > > /* stopping ftrace calls (just send to ftrace_stub) */ > > ftrace_trace_function = ftrace_stub; > > > > ftrace_shutdown_sysctl(); > > } > > > > + last_ftrace_enabled = !!ftrace_enabled; > > out: > > And move the assignment of last_ftrace_enabled to after the "out:" > label. This change might make sense anyway. But it is not strictly necessary from my POV. Best Regards, Petr
