lkml.org 
[lkml]   [2009]   [Feb]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH tip 1/1] tracing: Only drop the BKL in register_tracer if there is a selftest
On Thu, Feb 12, 2009 at 05:23:54PM -0200, Arnaldo Carvalho de Melo wrote:
> Impact: cleanup
>
> We only need to drop the BKL for some tracers, and then only if the
> tracer being registered has a selftest method. So clean up moving the
> selftest bits to a different function.
>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Frédéric Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> kernel/trace/trace.c | 138 ++++++++++++++++++++++++++++----------------------
> 1 files changed, 77 insertions(+), 61 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 95f99a7..fc01e0c 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -452,6 +452,76 @@ update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
> __raw_spin_unlock(&ftrace_max_lock);
> }
>
> +static int trace_selftest(struct tracer *trace)
> +{
> + int ret = 0;
> +#ifdef CONFIG_FTRACE_STARTUP_TEST
> + struct tracer *saved_tracer;
> + struct trace_array *tr;
> + int i;
> +
> + if (trace->selftest == NULL || tracing_selftest_disabled)
> + return 0;
> + /*
> + * When this gets called we hold the BKL which means that
> + * preemption is disabled. Various trace selftests however
> + * need to disable and enable preemption for successful tests.
> + * So we drop the BKL here and grab it after the tests again.
> + */
> + unlock_kernel();
> +
> + saved_tracer = current_trace;
> + tr = &global_trace;
> +
> + /*
> + * Run a selftest on this tracer.
> + * Here we reset the trace buffer, and set the current
> + * tracer to be this tracer. The tracer can then run some
> + * internal tracing to verify that everything is in order.
> + * If we fail, we do not register this tracer.
> + */
> + for_each_tracing_cpu(i)
> + tracing_reset(tr, i);
> +
> + current_trace = trace;
> + /* the test is responsible for initializing and enabling */
> + pr_info("Testing tracer %s: ", trace->name);
> + ret = trace->selftest(trace, tr);
> + /* the test is responsible for resetting too */
> + current_trace = saved_tracer;
> + if (ret)
> + printk(KERN_CONT "FAILED!\n");
> + else {
> + /* Only reset on passing, to avoid touching corrupted buffers */
> + for_each_tracing_cpu(i)
> + tracing_reset(tr, i);
> +
> + printk(KERN_CONT "PASSED\n");
> + }
> +
> + tracing_selftest_running = false;
> + lock_kernel();
> +#endif
> + return ret;
> +}
> +
> +static void trace_boot(struct tracer *trace)
> +{
> + if (strncmp(default_bootup_tracer, trace->name, BOOTUP_TRACER_SIZE))
> + return;
> +
> + pr_info("Starting tracer '%s'\n", trace->name);
> + /* Do we want this tracer to start on bootup? */
> + tracing_set_tracer(trace->name);
> + default_bootup_tracer = NULL;
> + /* disable other selftests, since this will break it. */
> + tracing_selftest_disabled = 1;
> +#ifdef CONFIG_FTRACE_STARTUP_TEST
> + pr_info("Disabling FTRACE selftests due to running tracer '%s'\n",
> + trace->name);
> +#endif
> +}
> +
> /**
> * register_tracer - register a tracer with the ftrace system.
> * @type - the plugin for the tracer
> @@ -471,13 +541,6 @@ __acquires(kernel_lock)
> return -1;
> }
>
> - /*
> - * When this gets called we hold the BKL which means that
> - * preemption is disabled. Various trace selftests however
> - * need to disable and enable preemption for successful tests.
> - * So we drop the BKL here and grab it after the tests again.
> - */
> - unlock_kernel();
> mutex_lock(&trace_types_lock);
>
> tracing_selftest_running = true;
> @@ -488,7 +551,7 @@ __acquires(kernel_lock)
> pr_info("Trace %s already registered\n",
> type->name);
> ret = -1;
> - goto out;
> + goto out_mutex_unlock;
> }
> }
>
> @@ -500,39 +563,9 @@ __acquires(kernel_lock)
> if (!type->flags->opts)
> type->flags->opts = dummy_tracer_opt;
>
> -#ifdef CONFIG_FTRACE_STARTUP_TEST
> - if (type->selftest && !tracing_selftest_disabled) {
> - struct tracer *saved_tracer = current_trace;
> - struct trace_array *tr = &global_trace;
> - int i;
> -
> - /*
> - * Run a selftest on this tracer.
> - * Here we reset the trace buffer, and set the current
> - * tracer to be this tracer. The tracer can then run some
> - * internal tracing to verify that everything is in order.
> - * If we fail, we do not register this tracer.
> - */
> - for_each_tracing_cpu(i)
> - tracing_reset(tr, i);
> -
> - current_trace = type;
> - /* the test is responsible for initializing and enabling */
> - pr_info("Testing tracer %s: ", type->name);
> - ret = type->selftest(type, tr);
> - /* the test is responsible for resetting too */
> - current_trace = saved_tracer;
> - if (ret) {
> - printk(KERN_CONT "FAILED!\n");
> - goto out;
> - }
> - /* Only reset on passing, to avoid touching corrupted buffers */
> - for_each_tracing_cpu(i)
> - tracing_reset(tr, i);
> -
> - printk(KERN_CONT "PASSED\n");
> - }
> -#endif
> + ret = trace_selftest(type);
> + if (ret != 0)
> + goto out_mutex_unlock;
>
> type->next = trace_types;
> trace_types = type;
> @@ -540,29 +573,12 @@ __acquires(kernel_lock)
> if (len > max_tracer_type_len)
> max_tracer_type_len = len;
>
> - out:
> - tracing_selftest_running = false;
> +out_mutex_unlock:
> mutex_unlock(&trace_types_lock);
>
> - if (ret || !default_bootup_tracer)
> - goto out_unlock;
> -
> - if (strncmp(default_bootup_tracer, type->name, BOOTUP_TRACER_SIZE))
> - goto out_unlock;
> -
> - printk(KERN_INFO "Starting tracer '%s'\n", type->name);
> - /* Do we want this tracer to start on bootup? */
> - tracing_set_tracer(type->name);
> - default_bootup_tracer = NULL;
> - /* disable other selftests, since this will break it. */
> - tracing_selftest_disabled = 1;
> -#ifdef CONFIG_FTRACE_STARTUP_TEST
> - printk(KERN_INFO "Disabling FTRACE selftests due to running tracer '%s'\n",
> - type->name);
> -#endif
> + if (ret == 0 && default_bootup_tracer)
> + trace_boot(type);
>
> - out_unlock:
> - lock_kernel();
> return ret;
> }
>
> --
> 1.6.0.6
>

Yes, this is better like that.
Thanks.

--
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/

\
 
 \ /
  Last update: 2009-02-12 20:41    [W:0.036 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site