[for-linus][PATCH 02/13] tracing: Dump stacktrace trigger to the corresponding instance

[for-linus][PATCH 02/13] tracing: Dump stacktrace trigger to the corresponding instance

[Steven Rostedt]

From: Daniel Bristot de Oliveira <bristot@kernel.org>

The stacktrace event trigger is not dumping the stacktrace to the instance
where it was enabled, but to the global "instance."

Use the private_data, pointing to the trigger file, to figure out the
corresponding trace instance, and use it in the trigger action, like
snapshot_trigger does.

Link: https://lkml.kernel.org/r/afbb0b4f18ba92c276865bc97204d438473f4ebc.1645396236.git.bristot@kernel.org

Cc: stable@vger.kernel.org
Fixes: ae63b31e4d0e2 ("tracing: Separate out trace events from global variables")
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Tested-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_trigger.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index d00fee705f9c..e0d50c9577f3 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -1540,7 +1540,12 @@ stacktrace_trigger(struct event_trigger_data *data,
 		   struct trace_buffer *buffer,  void *rec,
 		   struct ring_buffer_event *event)
 {
-	trace_dump_stack(STACK_SKIP);
+	struct trace_event_file *file = data->private_data;
+
+	if (file)
+		__trace_stack(file->tr, tracing_gen_ctx(), STACK_SKIP);
+	else
+		trace_dump_stack(STACK_SKIP);
 }
 
 static void
-- 
2.34.1

[for-linus][PATCH 03/13] tracing: Have traceon and traceoff trigger honor the instance

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

If a trigger is set on an event to disable or enable tracing within an
instance, then tracing should be disabled or enabled in the instance and
not at the top level, which is confusing to users.

Link: https://lkml.kernel.org/r/20220223223837.14f94ec3@rorschach.local.home

Cc: Tom Zanussi <zanussi@kernel.org>
Cc: stable@vger.kernel.org
Fixes: ae63b31e4d0e2 ("tracing: Separate out trace events from global variables")
Tested-by: Daniel Bristot de Oliveira <bristot@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_trigger.c | 52 +++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index e0d50c9577f3..efe563140f27 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -1295,6 +1295,16 @@ traceon_trigger(struct event_trigger_data *data,
 		struct trace_buffer *buffer, void *rec,
 		struct ring_buffer_event *event)
 {
+	struct trace_event_file *file = data->private_data;
+
+	if (file) {
+		if (tracer_tracing_is_on(file->tr))
+			return;
+
+		tracer_tracing_on(file->tr);
+		return;
+	}
+
 	if (tracing_is_on())
 		return;
 
@@ -1306,8 +1316,15 @@ traceon_count_trigger(struct event_trigger_data *data,
 		      struct trace_buffer *buffer, void *rec,
 		      struct ring_buffer_event *event)
 {
-	if (tracing_is_on())
-		return;
+	struct trace_event_file *file = data->private_data;
+
+	if (file) {
+		if (tracer_tracing_is_on(file->tr))
+			return;
+	} else {
+		if (tracing_is_on())
+			return;
+	}
 
 	if (!data->count)
 		return;
@@ -1315,7 +1332,10 @@ traceon_count_trigger(struct event_trigger_data *data,
 	if (data->count != -1)
 		(data->count)--;
 
-	tracing_on();
+	if (file)
+		tracer_tracing_on(file->tr);
+	else
+		tracing_on();
 }
 
 static void
@@ -1323,6 +1343,16 @@ traceoff_trigger(struct event_trigger_data *data,
 		 struct trace_buffer *buffer, void *rec,
 		 struct ring_buffer_event *event)
 {
+	struct trace_event_file *file = data->private_data;
+
+	if (file) {
+		if (!tracer_tracing_is_on(file->tr))
+			return;
+
+		tracer_tracing_off(file->tr);
+		return;
+	}
+
 	if (!tracing_is_on())
 		return;
 
@@ -1334,8 +1364,15 @@ traceoff_count_trigger(struct event_trigger_data *data,
 		       struct trace_buffer *buffer, void *rec,
 		       struct ring_buffer_event *event)
 {
-	if (!tracing_is_on())
-		return;
+	struct trace_event_file *file = data->private_data;
+
+	if (file) {
+		if (!tracer_tracing_is_on(file->tr))
+			return;
+	} else {
+		if (!tracing_is_on())
+			return;
+	}
 
 	if (!data->count)
 		return;
@@ -1343,7 +1380,10 @@ traceoff_count_trigger(struct event_trigger_data *data,
 	if (data->count != -1)
 		(data->count)--;
 
-	tracing_off();
+	if (file)
+		tracer_tracing_off(file->tr);
+	else
+		tracing_off();
 }
 
 static int
-- 
2.34.1

[for-linus][PATCH 09/13] tracefs: Set the group ownership in apply_options() not parse_options()

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Al Viro brought it to my attention that the dentries may not be filled
when the parse_options() is called, causing the call to set_gid() to
possibly crash. It should only be called if parse_options() succeeds
totally anyway.

He suggested the logical place to do the update is in apply_options().

Cc: stable@vger.kernel.org
Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Fixes: 48b27b6b5191 ("tracefs: Set all files to the same group ownership as the mount option")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 fs/tracefs/inode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index bafc02bf8220..3638d330ff5a 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -264,7 +264,6 @@ static int tracefs_parse_options(char *data, struct tracefs_mount_opts *opts)
 			if (!gid_valid(gid))
 				return -EINVAL;
 			opts->gid = gid;
-			set_gid(tracefs_mount->mnt_root, gid);
 			break;
 		case Opt_mode:
 			if (match_octal(&args[0], &option))
@@ -293,6 +292,9 @@ static int tracefs_apply_options(struct super_block *sb)
 	inode->i_uid = opts->uid;
 	inode->i_gid = opts->gid;
 
+	if (tracefs_mount && tracefs_mount->mnt_root)
+		set_gid(tracefs_mount->mnt_root, opts->gid);
+
 	return 0;
 }
 
-- 
2.34.1

Re: [for-linus][PATCH 09/13] tracefs: Set the group ownership in apply_options() not parse_options()

[Al Viro]

On Fri, Feb 25, 2022 at 11:52:00AM -0500, Steven Rostedt wrote:

> @@ -293,6 +292,9 @@ static int tracefs_apply_options(struct super_block *sb)
>  	inode->i_uid = opts->uid;
>  	inode->i_gid = opts->gid;
>  
> +	if (tracefs_mount && tracefs_mount->mnt_root)
> +		set_gid(tracefs_mount->mnt_root, opts->gid);
> +

Umm...  Why bother with tracefs_mount here in the first place?
You have the superblock the operation is applied for - right in the
arguments.

Just make that
	set_gid(sb->s_root, opts->gid);
and be done with that.  Same place in tracefs_apply_options()...

Incidentally, I'm not sure you need to keep ->i_gid assignment - set_gid()
won't miss that inode, so...

Re: [for-linus][PATCH 09/13] tracefs: Set the group ownership in apply_options() not parse_options()

[Steven Rostedt]

On Fri, 25 Feb 2022 17:11:00 +0000
Al Viro <viro@zeniv.linux.org.uk> wrote:

> On Fri, Feb 25, 2022 at 11:52:00AM -0500, Steven Rostedt wrote:
> 
> > @@ -293,6 +292,9 @@ static int tracefs_apply_options(struct super_block *sb)
> >  	inode->i_uid = opts->uid;
> >  	inode->i_gid = opts->gid;
> >  
> > +	if (tracefs_mount && tracefs_mount->mnt_root)
> > +		set_gid(tracefs_mount->mnt_root, opts->gid);
> > +  
> 
> Umm...  Why bother with tracefs_mount here in the first place?
> You have the superblock the operation is applied for - right in the
> arguments.

Ah, because I didn't realize they were one and the same ;-)

> 
> Just make that
> 	set_gid(sb->s_root, opts->gid);
> and be done with that.  Same place in tracefs_apply_options()...
> 
> Incidentally, I'm not sure you need to keep ->i_gid assignment - set_gid()
> won't miss that inode, so...

Makes sense. Thanks.

I guess I wont be sending my pull request to Linus now :-/

-- Steve