Messages in this thread | | | Date | Mon, 2 Feb 2009 20:20:41 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH 2/3] trace: fix default boot up tracer |
| |
On Mon, 2 Feb 2009 23:12:37 -0500 (EST) Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Mon, 2 Feb 2009, Andrew Morton wrote: > > > On Mon, 02 Feb 2009 21:38:32 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > @@ -510,8 +520,25 @@ int register_tracer(struct tracer *type) > > > out: > > > tracing_selftest_running = false; > > > mutex_unlock(&trace_types_lock); > > > - lock_kernel(); > > > > > > + if (!ret && default_bootup_tracer) { > > > + if (!strncmp(default_bootup_tracer, type->name, > > > + BOOTUP_TRACER_SIZE)) { > > > + 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 > > > + } > > > + } > > > + > > > + lock_kernel(); > > > return ret; > > > } > > > > The fun and games which register_tracer() plays with lock_kernel() tell > > us that this function is only called at bootup time and hence could be > > __init. A quick whizz through callers confirms that. > > > > And if register_tracer() is also only callable at bootup, one suspects > > that unregister_tracer() isn't useful. And lo, it has no callers. > > > > This leads one to further surmise that trace_types_lock a) could be > > __initdata and b) could be removed (the list is only altered when we're > > running single-threaded). This appears to be the case. > > The lock_kernel addition was added when the BKL became a spinlock again. > The selftests needed to be able to sleep, and this caused issues.
Sleeping inside lock_kernel() is quite OK. Confused.
What is the call path to this function? Does it all happen under ftrace_init()? If not, do we risk breaking start_kernel()'s thou-shalt-not-enable-interrupts-early rule which powerpc (at least) imposes?
> The register_tracer was initial written to be pluggable at any time. > Perhaps in the future to allow modules. But this does not seem to have > panned out. > > Since we have the lock_kernel there anyway, if we ever need to handle > modules, that will need a different interface anyway. I guess I can nuke > the unregister tracer.
And add some __init/__initdatas?
> As for the trace_types_lock mutex, that is still needed. Not only did it > guard against the tracer registry, it also guards the switching of tracers. > > # echo function > /debug/tracing/current_tracer >
OK, it's a dual-use lock, as the comment indicates.
| |