[PATCH] RFC: Add signal command to trace events

[PATCH] RFC: Add signal command to trace events

[Christopher Covington]

This stops a process after an event has been triggered a given
number of times. For example:

[[ -d /sys/kernel/debug/tracing ]] || \
  mount -t debugfs none /sys/kernel/debug
echo "p:myevent $(which myapp):0x700" > /sys/kernel/debug/tracing/uprobe_events
echo 1 > /sys/kernel/debug/tracing/events/uprobes/myevent/enable
echo 'signal:3' > /sys/kernel/debug/tracing/events/uprobes/myevent/trigger
myapp
myapp
myapp &

Here, the 0x700 offset is the entry to main(). On the third
invocation of the program, the process is stopped.

Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 include/linux/ftrace_event.h        |  1 +
 kernel/trace/trace_events_trigger.c | 73 +++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index cff3106..7bca0df 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -402,6 +402,7 @@ enum event_trigger_type {
 	ETT_SNAPSHOT		= (1 << 1),
 	ETT_STACKTRACE		= (1 << 2),
 	ETT_EVENT_ENABLE	= (1 << 3),
+	ETT_SIGNAL		= (1 << 4),
 };
 
 extern void destroy_preds(struct ftrace_event_file *file);
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 4747b47..d0599fa 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -1042,6 +1042,78 @@ static __init int register_trigger_stacktrace_cmd(void)
 static __init int register_trigger_stacktrace_cmd(void) { return 0; }
 #endif /* CONFIG_STACKTRACE */
 
+//ifdef CONFIG_TRACER_SIGNAL
+static void
+signal_trigger(struct event_trigger_data *data)
+{
+	force_sig(SIGSTOP, current);
+}
+
+static void
+signal_count_trigger(struct event_trigger_data *data)
+{
+	switch(data->count) {
+	case 0:
+		return;
+	case 1:
+		signal_trigger(data);
+		// fall through
+	default:
+		(data->count)--;
+	}
+}
+
+static int
+signal_trigger_print(struct seq_file *m, struct event_trigger_ops *ops,
+		       struct event_trigger_data *data)
+{
+	return event_trigger_print("signal", m, (void *)data->count,
+				   data->filter_str);
+}
+
+static struct event_trigger_ops signal_trigger_ops = {
+	.func			= signal_trigger,
+	.print			= signal_trigger_print,
+	.init			= event_trigger_init,
+	.free			= event_trigger_free,
+};
+
+static struct event_trigger_ops signal_count_trigger_ops = {
+	.func			= signal_count_trigger,
+	.print			= signal_trigger_print,
+	.init			= event_trigger_init,
+	.free			= event_trigger_free,
+};
+
+static struct event_trigger_ops *
+signal_get_trigger_ops(char *cmd, char *param)
+{
+	return param ? &signal_count_trigger_ops : &signal_trigger_ops;
+}
+
+static struct event_command trigger_signal_cmd = {
+	.name			= "signal",
+	.trigger_type		= ETT_SIGNAL,
+	.func			= event_trigger_callback,
+	.reg			= register_trigger,
+	.unreg			= unregister_trigger,
+	.get_trigger_ops	= signal_get_trigger_ops,
+	.set_filter		= set_trigger_filter,
+};
+
+static __init int register_trigger_signal_cmd(void)
+{
+	int ret;
+
+	ret = register_event_command(&trigger_signal_cmd);
+	WARN_ON(ret < 0);
+
+	return ret;
+}
+/* else
+static __init int register_trigger_signal_cmd(void) { return 0; }
+endif CONFIG_TRACER_SIGNAL */
+
 static __init void unregister_trigger_traceon_traceoff_cmds(void)
 {
 	unregister_event_command(&trigger_traceon_cmd);
@@ -1432,6 +1504,7 @@ __init int register_trigger_cmds(void)
 	register_trigger_snapshot_cmd();
 	register_trigger_stacktrace_cmd();
 	register_trigger_enable_disable_cmds();
+	register_trigger_signal_cmd();
 
 	return 0;
 }
-- 
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by the Linux Foundation.

Re: [PATCH] RFC: Add signal command to trace events

[Steven Rostedt]

[ Added Oleg who is the guru of signals ;-) ]

On Thu, 26 Jun 2014 09:57:21 -0400
Christopher Covington <cov@codeaurora.org> wrote:

> This stops a process after an event has been triggered a given
> number of times. For example:
> 
> [[ -d /sys/kernel/debug/tracing ]] || \
>   mount -t debugfs none /sys/kernel/debug
> echo "p:myevent $(which myapp):0x700" > /sys/kernel/debug/tracing/uprobe_events
> echo 1 > /sys/kernel/debug/tracing/events/uprobes/myevent/enable
> echo 'signal:3' > /sys/kernel/debug/tracing/events/uprobes/myevent/trigger
> myapp
> myapp
> myapp &
> 
> Here, the 0x700 offset is the entry to main(). On the third
> invocation of the program, the process is stopped.
> 

Interesting trigger. I can see this being useful.

> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  include/linux/ftrace_event.h        |  1 +
>  kernel/trace/trace_events_trigger.c | 73 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index cff3106..7bca0df 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -402,6 +402,7 @@ enum event_trigger_type {
>  	ETT_SNAPSHOT		= (1 << 1),
>  	ETT_STACKTRACE		= (1 << 2),
>  	ETT_EVENT_ENABLE	= (1 << 3),
> +	ETT_SIGNAL		= (1 << 4),
>  };
>  
>  extern void destroy_preds(struct ftrace_event_file *file);
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index 4747b47..d0599fa 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -1042,6 +1042,78 @@ static __init int register_trigger_stacktrace_cmd(void)
>  static __init int register_trigger_stacktrace_cmd(void) { return 0; }
>  #endif /* CONFIG_STACKTRACE */
>  
> +//ifdef CONFIG_TRACER_SIGNAL
> +static void
> +signal_trigger(struct event_trigger_data *data)
> +{
> +	force_sig(SIGSTOP, current);

Here's the issue though. Events can happen most anywhere. An event
context you need to be very careful what you do. force_sig() is not a
light weight function by any means. Basically, anything that grabs
spinlocks must be extremely careful, because you can have an event in
most functions and you may have just introduced a inverse locking
order. Can you take the signal spinlock when holding the rq lock?
There's several tracepoints that are called while holding the rq lock.

But don't fret. All is not lost.

What can be done is use irq_work instead. This is very useful as well.
Although it will have a delayed effect. Maybe you can do the following:

	if (irqs_disabled())
		irq_work_queue(...);
	else
		force_sig(...);

That is, if interrupts are enabled, it is pretty much guaranteed that
calling force_sig() is safe. Heck, interrupts can do it. If not, then
we need to delay the action, because it may not be safe. Unfortunately,
I believe uprobe tracepoints are called with interrupts diasbled (at
least it shows that in my trace of a uprobe).

-- Steve

> +}
> +
> +static void
> +signal_count_trigger(struct event_trigger_data *data)
> +{
> +	switch(data->count) {
> +	case 0:
> +		return;
> +	case 1:
> +		signal_trigger(data);
> +		// fall through
> +	default:
> +		(data->count)--;
> +	}
> +}
> +
> +static int
> +signal_trigger_print(struct seq_file *m, struct event_trigger_ops *ops,
> +		       struct event_trigger_data *data)
> +{
> +	return event_trigger_print("signal", m, (void *)data->count,
> +				   data->filter_str);
> +}
> +
> +static struct event_trigger_ops signal_trigger_ops = {
> +	.func			= signal_trigger,
> +	.print			= signal_trigger_print,
> +	.init			= event_trigger_init,
> +	.free			= event_trigger_free,
> +};
> +
> +static struct event_trigger_ops signal_count_trigger_ops = {
> +	.func			= signal_count_trigger,
> +	.print			= signal_trigger_print,
> +	.init			= event_trigger_init,
> +	.free			= event_trigger_free,
> +};
> +
> +static struct event_trigger_ops *
> +signal_get_trigger_ops(char *cmd, char *param)
> +{
> +	return param ? &signal_count_trigger_ops : &signal_trigger_ops;
> +}
> +
> +static struct event_command trigger_signal_cmd = {
> +	.name			= "signal",
> +	.trigger_type		= ETT_SIGNAL,
> +	.func			= event_trigger_callback,
> +	.reg			= register_trigger,
> +	.unreg			= unregister_trigger,
> +	.get_trigger_ops	= signal_get_trigger_ops,
> +	.set_filter		= set_trigger_filter,
> +};
> +
> +static __init int register_trigger_signal_cmd(void)
> +{
> +	int ret;
> +
> +	ret = register_event_command(&trigger_signal_cmd);
> +	WARN_ON(ret < 0);
> +
> +	return ret;
> +}
> +/* else
> +static __init int register_trigger_signal_cmd(void) { return 0; }
> +endif CONFIG_TRACER_SIGNAL */
> +
>  static __init void unregister_trigger_traceon_traceoff_cmds(void)
>  {
>  	unregister_event_command(&trigger_traceon_cmd);
> @@ -1432,6 +1504,7 @@ __init int register_trigger_cmds(void)
>  	register_trigger_snapshot_cmd();
>  	register_trigger_stacktrace_cmd();
>  	register_trigger_enable_disable_cmds();
> +	register_trigger_signal_cmd();
>  
>  	return 0;
>  }

Re: [PATCH] RFC: Add signal command to trace events

[Oleg Nesterov]

On 06/26, Steven Rostedt wrote:
>
> [ Added Oleg who is the guru of signals ;-) ]

Thanks,

I didn't see the patch, so I can only guess what it does,

> On Thu, 26 Jun 2014 09:57:21 -0400
> Christopher Covington <cov@codeaurora.org> wrote:
>
> > This stops a process after an event has been triggered a given
> > number of times. For example:
> >
> > [[ -d /sys/kernel/debug/tracing ]] || \
> >   mount -t debugfs none /sys/kernel/debug
> > echo "p:myevent $(which myapp):0x700" > /sys/kernel/debug/tracing/uprobe_events
> > echo 1 > /sys/kernel/debug/tracing/events/uprobes/myevent/enable
> > echo 'signal:3' > /sys/kernel/debug/tracing/events/uprobes/myevent/trigger
> > myapp
> > myapp
> > myapp &
> >
> > Here, the 0x700 offset is the entry to main(). On the third
> > invocation of the program, the process is stopped.
> >
>
> Interesting trigger. I can see this being useful.

Yes. I thought about this too.

> > +//ifdef CONFIG_TRACER_SIGNAL
> > +static void
> > +signal_trigger(struct event_trigger_data *data)
> > +{
> > +	force_sig(SIGSTOP, current);

Well, I don't like this. Imho, it would be nice to avoid signals here.

And SIGSTOP in particular. It can interfere with the "real" SIGSTOP
sent by user, with ptrace, with tty.

I think we need something else... "STOP" is actually simple, but we need
some interface which allows to wakeup a task sleeping in TASK_KILLABLE
after it hits ETT_FREEZE.

Oleg.

Re: [PATCH] RFC: Add signal command to trace events

[Oleg Nesterov]

On 06/26, Oleg Nesterov wrote:
>
> > > +//ifdef CONFIG_TRACER_SIGNAL
> > > +static void
> > > +signal_trigger(struct event_trigger_data *data)
> > > +{
> > > +	force_sig(SIGSTOP, current);
>
> Well, I don't like this. Imho, it would be nice to avoid signals here.
>
> And SIGSTOP in particular. It can interfere with the "real" SIGSTOP
> sent by user, with ptrace, with tty.

And I forgot to mention, note that SIGSTOP stops the whole thread group,
this is not what we want.

> I think we need something else... "STOP" is actually simple, but we need
> some interface which allows to wakeup a task sleeping in TASK_KILLABLE
> after it hits ETT_FREEZE.

Yes...

Oleg.

Re: [PATCH] RFC: Add signal command to trace events

[Steven Rostedt]

On Thu, 26 Jun 2014 18:29:43 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

> On 06/26, Steven Rostedt wrote:
> >
> > [ Added Oleg who is the guru of signals ;-) ]
> 
> Thanks,
> 
> I didn't see the patch, so I can only guess what it does,
> 

I didn't crop the patch on my reply. But if you want me to bounce it to
you I can.

-- Steve

Re: [PATCH] RFC: Add signal command to trace events

[Oleg Nesterov]

On 06/26, Steven Rostedt wrote:
>
> On Thu, 26 Jun 2014 18:29:43 +0200
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > On 06/26, Steven Rostedt wrote:
> > >
> > > [ Added Oleg who is the guru of signals ;-) ]
> >
> > Thanks,
> >
> > I didn't see the patch, so I can only guess what it does,
> >
>
> I didn't crop the patch on my reply. But if you want me to bounce it to
> you I can.

I was going to find it on lkml.org, but yes, thanks, please forward it to me ;)

Oleg.