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]

Message-ID: <1324323335.5916.46.camel@gandalf.stny.rr.com>
Date:	Mon, 19 Dec 2011 14:35:35 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Jiri Olsa <jolsa@...hat.com>
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 Mon, 2011-12-05 at 18:22 +0100, Jiri Olsa wrote:
>  
> +static inline void enable_ftrace_function(struct ftrace_ops *ops)
> +{
> +	atomic_t *disabled = this_cpu_ptr(ops->disabled);
> +	atomic_dec(disabled);
> +}
> +
> +static inline void disable_ftrace_function(struct ftrace_ops *ops)
> +{
> +	atomic_t *disabled = this_cpu_ptr(ops->disabled);
> +	atomic_inc(disabled);
> +}
> +

The above should be renamed to ftrace_function_enable/disable(), and
they should pass in the cpu. There may be a case we want to disable
ftrace functions on another CPU.

Not to mention, this is the perfect example of "this_cpu_ptr" being used
incorrectly. It's not made for this purpose, and again the naming of
"this_cpu" totally confuses other kernel developers. We need to change
this name as it was agreed to at Kernel Summit.

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.

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