linux-kernel - Re: [PATCH 2/3] trace: fix default boot up tracer

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 2 Feb 2009 23:12:37 -0500 (EST)
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <peterz@...radead.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Arjan van de Ven <arjan@...radead.org>,
	Steven Rostedt <srostedt@...hat.com>
Subject: Re: [PATCH 2/3] trace: fix default boot up tracer


On Mon, 2 Feb 2009, Andrew Morton wrote:

> On Mon, 02 Feb 2009 21:38:32 -0500 Steven Rostedt <rostedt@...dmis.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.

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.

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

-- Steve


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