| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20090212193751.GA5003@nowhere>
Date: Thu, 12 Feb 2009 20:37:53 +0100
From: Frederic Weisbecker <fweisbec@...il.com>
To: Arnaldo Carvalho de Melo <acme@...stprotocols.net>
Cc: Steven Rostedt <rostedt@...dmis.org>, Ingo Molnar <mingo@...e.hu>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [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@...e.hu>
> Cc: Frédéric Weisbecker <fweisbec@...il.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists