linux-kernel - Re: [PATCH 3/8] pstore: Add persistent function tracing

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]

Message-ID: <20120717200130.GA31678@lizard>
Date:	Tue, 17 Jul 2012 13:01:30 -0700
From:	Anton Vorontsov <anton.vorontsov@...aro.org>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Kees Cook <keescook@...omium.org>,
	Colin Cross <ccross@...roid.com>,
	Tony Luck <tony.luck@...el.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>, Arnd Bergmann <arnd@...db.de>,
	John Stultz <john.stultz@...aro.org>,
	Shuah Khan <shuahkhan@...il.com>, arve@...roid.com,
	Rebecca Schultz Zavin <rebecca@...roid.com>,
	Jesper Juhl <jj@...osbits.net>,
	Randy Dunlap <rdunlap@...otime.net>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Thomas Meyer <thomas@...3r.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Marco Stornelli <marco.stornelli@...il.com>,
	WANG Cong <xiyou.wangcong@...il.com>,
	linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
	linaro-kernel@...ts.linaro.org, patches@...aro.org,
	kernel-team@...roid.com
Subject: Re: [PATCH 3/8] pstore: Add persistent function tracing

On Tue, Jul 17, 2012 at 03:38:18PM -0400, Steven Rostedt wrote:
[...]
> > +void notrace pstore_ftrace_call(unsigned long ip, unsigned long parent_ip)
> 
> BTW, you can make the entire file 'notrace' without adding annotations
> by including in the Makefile:
> 
> CFLAGS_REMOVE_ftrace.o = -pg

Actually it was in the first version in the patch, but then I changed
it 'notrace' for just this func. This is for the case if the file would
contain some more code which we actually may trace. Doing things fine-
grained seemed to be better than making the whole file as notrace. Plus
it is one line less. :-)

But I have no preference, so I can change it.

> > +{
> > +	struct pstore_ftrace_record rec = {};
> > +
> > +	if (unlikely(oops_in_progress))
> > +		return;
> > +
> > +	rec.ip = ip;
> > +	rec.parent_ip = parent_ip;
> > +	pstore_ftrace_encode_cpu(&rec, raw_smp_processor_id());
> > +	psinfo->write_buf(PSTORE_TYPE_FTRACE, 0, NULL, 0, (void *)&rec,
> > +			  sizeof(rec), psinfo);
> > +}
> 
> BTW, can any of the called functions go into module code that can be
> removed? If so, then this is not safe at all. Normal function tracing
> can not be synced in a preemptible kernel.

Um. Yes, psinfo->write_buf() might be in the module. Nice catch.

> Also, I'm starting to wonder if this should be in its own utility
> (separate debugfs?) than hooking directly into ftrace. Then you don't
> need to modify ftrace at all and you can do the following:
> 
> static struct ftrace_ops trace_ops {
> 	.func = pstore_ftrace_call;
> };
> 
> then in your write to debugfs file:
> 
> 	register_ftrace_function(&trace_ops);
> 
> To turn off tracing:
> 
> 	unregister_ftrace_function(&trace_ops);
> 
> Note, it's safe to use if the trace_ops (or anything the callback calls)
> is a module. That's because it detects the trace_ops is not kernel core
> code and will place a wrapper around it that allows the function tracing
> to by synced with module unload. You still need to unregister the
> trace_ops before unloading the module, or you can have a crash that way.

Hehe. Like this? http://lkml.org/lkml/2012/5/26/80 :-D

So, do you want something like this, but combinded: we don't register
another tracer, but register our own ftrace_ops? This sounds doable.

Thanks!

-- 
Anton Vorontsov
Email: cbouatmailru@...il.com
--
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/