From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753511AbZBCEVY (ORCPT ); Mon, 2 Feb 2009 23:21:24 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751637AbZBCEVP (ORCPT ); Mon, 2 Feb 2009 23:21:15 -0500 Received: from smtp1.linux-foundation.org ([140.211.169.13]:48966 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751535AbZBCEVP (ORCPT ); Mon, 2 Feb 2009 23:21:15 -0500 Date: Mon, 2 Feb 2009 20:20:41 -0800 From: Andrew Morton To: Steven Rostedt Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Frederic Weisbecker , Arjan van de Ven , Steven Rostedt Subject: Re: [PATCH 2/3] trace: fix default boot up tracer Message-Id: <20090202202041.6256a566.akpm@linux-foundation.org> In-Reply-To: References: <20090203023830.440860800@goodmis.org> <20090203024358.332574797@goodmis.org> <20090202195021.83bd0b80.akpm@linux-foundation.org> X-Mailer: Sylpheed 2.4.8 (GTK+ 2.12.5; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2 Feb 2009 23:12:37 -0500 (EST) Steven Rostedt wrote: > > On Mon, 2 Feb 2009, Andrew Morton wrote: > > > On Mon, 02 Feb 2009 21:38:32 -0500 Steven Rostedt 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.