linux-kernel - Re: [PATCHv2 03/10] ftrace: Add enable/disable ftrace

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:	Tue, 20 Dec 2011 16:35:06 +0100
From:	Jiri Olsa <jolsa@...hat.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	fweisbec@...il.com, mingo@...hat.com, paulus@...ba.org,
	acme@...stprotocols.net, a.p.zijlstra@...llo.nl,
	linux-kernel@...r.kernel.org, aarapov@...hat.com,
	Christoph Lameter <christoph@...eter.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCHv2 03/10] ftrace: Add enable/disable ftrace_ops control
 interface

On Tue, Dec 20, 2011 at 10:25:53AM -0500, Steven Rostedt wrote:
> On Tue, 2011-12-20 at 15:57 +0100, Jiri Olsa wrote:
> 
> > > 
> > > If the above is called with preemption enabled, it will not do what is
> > > expected. We could disable function tracing on one CPU and then
> > > re-enable it for another CPU even though it is already enabled.
> > 
> > It is only called inside perf reg callback within the
> > schedule function where the preemption is disabled.
> > 
> > The ftrace_function_enable is called when task is scheduled in
> > on respective cpu. Likewise the ftrace_function_disable is called 
> > when task is scheduled out on respective cpu.
> 
> Yes I know how you use it, but this is an open API. It may be currently
> only used by perf today, but that doesn't mean that it wont be used by
> others. There's no documentation on how to use it. I don't look at this
> and say, "oh this is used by perf, we only need to worry about how perf
> uses it". That doesn't scale. It needs to be documented on how to use
> it, and if it requires preemption disabled when calling it, it should
> definitely be stated that fact and it may need a
> WARN_ON(preempt_count()) or something.

I've already added

        if (WARN_ON_ONCE(!(ops->flags & FTRACE_OPS_FL_CONTROL)))
                return;

will add similar for preemption and make comments for both functions

thanks,
jirka

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