public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Chunyu Hu <chuhu@redhat.com>
Subject: [for-next][PATCH 01/12] tracing: Make tracer_flags use the right set_flag callback
Date: Wed, 09 Mar 2016 09:46:26 -0500	[thread overview]
Message-ID: <20160309144744.201628969@goodmis.org> (raw)
In-Reply-To: 20160309144625.888964064@goodmis.org

[-- Attachment #1: 0001-tracing-Make-tracer_flags-use-the-right-set_flag-cal.patch --]
[-- Type: text/plain, Size: 4693 bytes --]

From: Chunyu Hu <chuhu@redhat.com>

When I was updating the ftrace_stress test of ltp. I encountered
a strange phenomemon, excute following steps:

echo nop > /sys/kernel/debug/tracing/current_tracer
echo 0 > /sys/kernel/debug/tracing/options/funcgraph-cpu
bash: echo: write error: Invalid argument

check dmesg:
[ 1024.903855] nop_test_refuse flag set to 0: we refuse.Now cat trace_options to see the result

The reason is that the trace option test will randomly setup trace
option under tracing/options no matter what the current_tracer is.
but the set_tracer_option is always using the set_flag callback
from the current_tracer. This patch adds a pointer to tracer_flags
and make it point to the tracer it belongs to. When the option is
setup, the set_flag of the right tracer will be used no matter
what the the current_tracer is.

And the old dummy_tracer_flags is used for all the tracers which
doesn't have a tracer_flags, having issue to use it to save the
pointer of a tracer. So remove it and use dynamic dummy tracer_flags
for tracers needing a dummy tracer_flags, as a result, there are no
tracers sharing tracer_flags, so remove the check code.

And save the current tracer to trace_option_dentry seems not good as
it may waste mem space when mount the debug/trace fs more than one time.

Link: http://lkml.kernel.org/r/1457444222-8654-1-git-send-email-chuhu@redhat.com

Signed-off-by: Chunyu Hu <chuhu@redhat.com>
[ Fixed up function tracer options to work with the change ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace.c           | 28 ++++++++++++++--------------
 kernel/trace/trace.h           |  1 +
 kernel/trace/trace_functions.c |  6 ++++++
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d9293402ee68..b401a1892dc6 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -74,11 +74,6 @@ static struct tracer_opt dummy_tracer_opt[] = {
 	{ }
 };
 
-static struct tracer_flags dummy_tracer_flags = {
-	.val = 0,
-	.opts = dummy_tracer_opt
-};
-
 static int
 dummy_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
 {
@@ -1258,12 +1253,20 @@ int __init register_tracer(struct tracer *type)
 
 	if (!type->set_flag)
 		type->set_flag = &dummy_set_flag;
-	if (!type->flags)
-		type->flags = &dummy_tracer_flags;
-	else
+	if (!type->flags) {
+		/*allocate a dummy tracer_flags*/
+		type->flags = kmalloc(sizeof(*type->flags), GFP_KERNEL);
+		if (!type->flags)
+			return -ENOMEM;
+		type->flags->val = 0;
+		type->flags->opts = dummy_tracer_opt;
+	} else
 		if (!type->flags->opts)
 			type->flags->opts = dummy_tracer_opt;
 
+	/* store the tracer for __set_tracer_option */
+	type->flags->trace = type;
+
 	ret = run_tracer_selftest(type);
 	if (ret < 0)
 		goto out;
@@ -3505,7 +3508,7 @@ static int __set_tracer_option(struct trace_array *tr,
 			       struct tracer_flags *tracer_flags,
 			       struct tracer_opt *opts, int neg)
 {
-	struct tracer *trace = tr->current_trace;
+	struct tracer *trace = tracer_flags->trace;
 	int ret;
 
 	ret = trace->set_flag(tr, tracer_flags->val, opts->bit, !neg);
@@ -6391,11 +6394,8 @@ create_trace_option_files(struct trace_array *tr, struct tracer *tracer)
 		return;
 
 	for (i = 0; i < tr->nr_topts; i++) {
-		/*
-		 * Check if these flags have already been added.
-		 * Some tracers share flags.
-		 */
-		if (tr->topts[i].tracer->flags == tracer->flags)
+		/* Make sure there's no duplicate flags. */
+		if (WARN_ON_ONCE(tr->topts[i].tracer->flags == tracer->flags))
 			return;
 	}
 
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 8414fa40bf27..b4cae47f283e 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -345,6 +345,7 @@ struct tracer_opt {
 struct tracer_flags {
 	u32			val;
 	struct tracer_opt	*opts;
+	struct tracer		*trace;
 };
 
 /* Makes more easy to define a tracer opt */
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index fcd41a166405..5a095c2e4b69 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -219,6 +219,8 @@ static void tracing_stop_function_trace(struct trace_array *tr)
 	unregister_ftrace_function(tr->ops);
 }
 
+static struct tracer function_trace;
+
 static int
 func_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
 {
@@ -228,6 +230,10 @@ func_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
 		if (!!set == !!(func_flags.val & TRACE_FUNC_OPT_STACK))
 			break;
 
+		/* We can change this flag when not running. */
+		if (tr->current_trace != &function_trace)
+			break;
+
 		unregister_ftrace_function(tr->ops);
 
 		if (set) {
-- 
2.7.0

  reply	other threads:[~2016-03-09 14:47 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-09 14:46 [for-next][PATCH 00/12] tracing: Various cleanups and preparation for trace event hist Steven Rostedt
2016-03-09 14:46 ` Steven Rostedt [this message]
2016-03-09 14:46 ` [for-next][PATCH 02/12] tracing: Remove duplicate checks for online CPUs Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 03/12] tracing: Make ftrace_event_field checking functions available Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 04/12] tracing: Make event trigger " Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 05/12] tracing: Add event record param to trigger_ops.func() Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 06/12] tracing: Add get_syscall_name() Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 07/12] tracing: Add a per-event-trigger paused field Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 08/12] tracing: Add needs_rec flag to event triggers Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 09/12] tracing: Add an unreg_all() callback to trigger commands Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 10/12] tracing: Use flags instead of bool in trigger structure Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 11/12] tracing, writeback: Replace cgroup path to cgroup ino Steven Rostedt
2016-03-09 14:46 ` [for-next][PATCH 12/12] tracing: Fix typoes in code comment and printk in trace_nop.c Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160309144744.201628969@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=chuhu@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox