On Thu, 18 Apr 2019 10:41:20 +0200
Thomas Gleixner <t...@linutronix.de> wrote:


> @@ -412,23 +404,20 @@ stack_trace_sysctl(struct ctl_table *tab
>                  void __user *buffer, size_t *lenp,
>                  loff_t *ppos)
>  {
> -     int ret;
> +     int ret, was_enabled;

One small nit. Could this be:

        int was_enabled;
        int ret;

I prefer only joining variables that are related on the same line.
Makes it look cleaner IMO.

>  
>       mutex_lock(&stack_sysctl_mutex);
> +     was_enabled = !!stack_tracer_enabled;
>  

Bah, not sure why I didn't do it this way to begin with. I think I
copied something else that couldn't do it this way for some reason and
didn't put any brain power behind the copy. :-/ But that was back in
2008 so I blame it on being "young and stupid" ;-)

Other then the above nit and removing the unneeded +1 in max_entries:

Reviewed-by: Steven Rostedt (VMware) <rost...@goodmis.org>

-- Steve


>       ret = proc_dointvec(table, write, buffer, lenp, ppos);
>  
> -     if (ret || !write ||
> -         (last_stack_tracer_enabled == !!stack_tracer_enabled))
> +     if (ret || !write || (was_enabled == !!stack_tracer_enabled))
>               goto out;
>  
> -     last_stack_tracer_enabled = !!stack_tracer_enabled;
> -
>       if (stack_tracer_enabled)
>               register_ftrace_function(&trace_ops);
>       else
>               unregister_ftrace_function(&trace_ops);
> -
>   out:
>       mutex_unlock(&stack_sysctl_mutex);
>       return ret;
> @@ -444,7 +433,6 @@ static __init int enable_stacktrace(char
>               strncpy(stack_trace_filter_buf, str + len, COMMAND_LINE_SIZE);
>  
>       stack_tracer_enabled = 1;
> -     last_stack_tracer_enabled = 1;
>       return 1;
>  }
>  __setup("stacktrace", enable_stacktrace);
> 

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to