2009-11-17 23:40:33

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 0/3] tracepoint: Add signal events

Hi,

These patches add signal related tracepoints including
signal generation, delivery, and loss. First patch also
moves signal-sending tracepoint from events/sched.h to
events/signal.h.

TODO:
- Add signal-wakeup event?

Thank you,

---

Masami Hiramatsu (3):
tracepoint: Add signal loss events
tracepoint: Add signal deliver event
tracepoint: Move signal sending tracepoint to events/signal.h


include/trace/events/sched.h | 25 --------
include/trace/events/signal.h | 133 +++++++++++++++++++++++++++++++++++++++++
kernel/signal.c | 27 ++++++--
3 files changed, 153 insertions(+), 32 deletions(-)
create mode 100644 include/trace/events/signal.h

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]


2009-11-17 23:41:33

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 1/3] tracepoint: Move signal sending tracepoint to events/signal.h

Move signal sending event to events/signal.h. This patch also renames
sched_signal_send event to signal_generate.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Roland McGrath <[email protected]>
Cc: Ingo Molnar <[email protected]>
---

include/trace/events/sched.h | 25 -------------------------
include/trace/events/signal.h | 38 ++++++++++++++++++++++++++++++++++++++
kernel/signal.c | 5 +++--
3 files changed, 41 insertions(+), 27 deletions(-)
create mode 100644 include/trace/events/signal.h

diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index b50b985..b221bb3 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -320,31 +320,6 @@ TRACE_EVENT(sched_process_fork,
);

/*
- * Tracepoint for sending a signal:
- */
-TRACE_EVENT(sched_signal_send,
-
- TP_PROTO(int sig, struct task_struct *p),
-
- TP_ARGS(sig, p),
-
- TP_STRUCT__entry(
- __field( int, sig )
- __array( char, comm, TASK_COMM_LEN )
- __field( pid_t, pid )
- ),
-
- TP_fast_assign(
- memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
- __entry->pid = p->pid;
- __entry->sig = sig;
- ),
-
- TP_printk("sig=%d comm=%s pid=%d",
- __entry->sig, __entry->comm, __entry->pid)
-);
-
-/*
* XXX the below sched_stat tracepoints only apply to SCHED_OTHER/BATCH/IDLE
* adding sched_stat support to SCHED_FIFO/RR would be welcome.
*/
diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
new file mode 100644
index 0000000..c7817f3
--- /dev/null
+++ b/include/trace/events/signal.h
@@ -0,0 +1,38 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM signal
+
+#if !defined(_TRACE_SIGNAL_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SIGNAL_H
+
+#include <linux/sched.h>
+#include <linux/tracepoint.h>
+
+/*
+ * Tracepoint for generating a signal:
+ */
+TRACE_EVENT(signal_generate,
+
+ TP_PROTO(int sig, struct task_struct *p),
+
+ TP_ARGS(sig, p),
+
+ TP_STRUCT__entry(
+ __field( int, sig )
+ __array( char, comm, TASK_COMM_LEN )
+ __field( pid_t, pid )
+ ),
+
+ TP_fast_assign(
+ memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
+ __entry->pid = p->pid;
+ __entry->sig = sig;
+ ),
+
+ TP_printk("sig=%d comm=%s pid=%d",
+ __entry->sig, __entry->comm, __entry->pid)
+);
+
+#endif /* _TRACE_SIGNAL_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/kernel/signal.c b/kernel/signal.c
index fe08008..82ab077 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -28,7 +28,8 @@
#include <linux/freezer.h>
#include <linux/pid_namespace.h>
#include <linux/nsproxy.h>
-#include <trace/events/sched.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/signal.h>

#include <asm/param.h>
#include <asm/uaccess.h>
@@ -856,7 +857,7 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
struct sigqueue *q;
int override_rlimit;

- trace_sched_signal_send(sig, t);
+ trace_signal_generate(sig, t);

assert_spin_locked(&t->sighand->siglock);



--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-11-18 00:38:26

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 2/3] tracepoint: Add signal deliver event

Add a tracepoint where a process gets a signal. This tracepoint
shows signal-number, sa-handler and sa-flag.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Roland McGrath <[email protected]>
Cc: Ingo Molnar <[email protected]>
---

include/trace/events/signal.h | 24 ++++++++++++++++++++++++
kernel/signal.c | 3 +++
2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
index c7817f3..6dfd88d 100644
--- a/include/trace/events/signal.h
+++ b/include/trace/events/signal.h
@@ -32,6 +32,30 @@ TRACE_EVENT(signal_generate,
__entry->sig, __entry->comm, __entry->pid)
);

+/*
+ * Tracepoint for delivering a signal:
+ */
+TRACE_EVENT(signal_deliver,
+
+ TP_PROTO(int sig, struct k_sigaction *ka),
+
+ TP_ARGS(sig, ka),
+
+ TP_STRUCT__entry(
+ __field( int, sig )
+ __field( unsigned long, sa_handler )
+ __field( unsigned long, sa_flags )
+ ),
+
+ TP_fast_assign(
+ __entry->sig = sig;
+ __entry->sa_handler = (unsigned long)ka->sa.sa_handler;
+ __entry->sa_flags = ka->sa.sa_flags;
+ ),
+
+ TP_printk("sig=%d sa_handler=%lx sa_flags=%lx",
+ __entry->sig, __entry->sa_handler, __entry->sa_flags)
+);
#endif /* _TRACE_SIGNAL_H */

/* This part must be outside protection */
diff --git a/kernel/signal.c b/kernel/signal.c
index 82ab077..5615f41 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1860,6 +1860,9 @@ relock:
ka = &sighand->action[signr-1];
}

+ /* Trace the actual delivered signals including SIG_IGN.*/
+ trace_signal_deliver(signr, ka);
+
if (ka->sa.sa_handler == SIG_IGN) /* Do nothing. */
continue;
if (ka->sa.sa_handler != SIG_DFL) {


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-11-17 23:41:17

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 3/3] tracepoint: Add signal loss events

Add signal_overflow_fail and signal_lose_info tracepoints
for signal-lost events.

Signed-off-by: Masami Hiramatsu <[email protected]>
Suggested-by: Roland McGrath <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Ingo Molnar <[email protected]>
---

include/trace/events/signal.h | 71 +++++++++++++++++++++++++++++++++++++++++
kernel/signal.c | 19 ++++++++---
2 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
index 6dfd88d..eb57285 100644
--- a/include/trace/events/signal.h
+++ b/include/trace/events/signal.h
@@ -4,6 +4,7 @@
#if !defined(_TRACE_SIGNAL_H) || defined(TRACE_HEADER_MULTI_READ)
#define _TRACE_SIGNAL_H

+#include <linux/signal.h>
#include <linux/sched.h>
#include <linux/tracepoint.h>

@@ -56,6 +57,76 @@ TRACE_EVENT(signal_deliver,
TP_printk("sig=%d sa_handler=%lx sa_flags=%lx",
__entry->sig, __entry->sa_handler, __entry->sa_flags)
);
+
+/*
+ * Tracepoint for losing a signal by overflow:
+ */
+TRACE_EVENT(signal_overflow_fail,
+
+ TP_PROTO(int sig, int group, struct siginfo *info),
+
+ TP_ARGS(sig, group, info),
+
+ TP_STRUCT__entry(
+ __field( int, sig )
+ __field( int, group )
+ __field( int, errno )
+ __field( int, code )
+ ),
+
+ TP_fast_assign(
+ __entry->sig = sig;
+ __entry->group = group;
+ if (info == SEND_SIG_NOINFO) {
+ __entry->errno = 0;
+ __entry->code = SI_USER;
+ } else if (info == SEND_SIG_PRIV) {
+ __entry->errno = 0;
+ __entry->code = SI_KERNEL;
+ } else {
+ __entry->errno = info->si_errno;
+ __entry->code = info->si_code;
+ }
+ ),
+
+ TP_printk("sig=%d group=%d errno=%d code=%d",
+ __entry->sig, __entry->group, __entry->errno, __entry->code)
+);
+
+/*
+ * Tracepoint for losing a siginfo:
+ */
+TRACE_EVENT(signal_lose_info,
+
+ TP_PROTO(int sig, int group, struct siginfo *info),
+
+ TP_ARGS(sig, group, info),
+
+ TP_STRUCT__entry(
+ __field( int, sig )
+ __field( int, group )
+ __field( int, errno )
+ __field( int, code )
+ ),
+
+ TP_fast_assign(
+ __entry->sig = sig;
+ __entry->group = group;
+ if (info == SEND_SIG_NOINFO) {
+ __entry->errno = 0;
+ __entry->code = SI_USER;
+ } else if (info == SEND_SIG_PRIV) {
+ __entry->errno = 0;
+ __entry->code = SI_KERNEL;
+ } else {
+ __entry->errno = info->si_errno;
+ __entry->code = info->si_code;
+ }
+ ),
+
+ TP_printk("sig=%d group=%d errno=%d code=%d",
+ __entry->sig, __entry->group, __entry->errno, __entry->code)
+);
#endif /* _TRACE_SIGNAL_H */

/* This part must be outside protection */
diff --git a/kernel/signal.c b/kernel/signal.c
index 5615f41..65f67c5 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -919,12 +919,21 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
break;
}
} else if (!is_si_special(info)) {
- if (sig >= SIGRTMIN && info->si_code != SI_USER)
- /*
- * Queue overflow, abort. We may abort if the signal was rt
- * and sent by user using something other than kill().
- */
+ if (sig >= SIGRTMIN && info->si_code != SI_USER) {
+ /*
+ * Queue overflow, abort. We may abort if the
+ * signal was rt and sent by user using something
+ * other than kill().
+ */
+ trace_signal_overflow_fail(sig, group, info);
return -EAGAIN;
+ } else {
+ /*
+ * This is a silent loss of information. We still
+ * send the signal, but the *info bits are lost.
+ */
+ trace_signal_lose_info(sig, group, info);
+ }
}

out_set:


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-11-18 03:07:50

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH -tip 0/3] tracepoint: Add signal events

I would add the siginfo_t * argument to each of these tracepoints.

In 2/3 this comment:

+ /* Trace the actual delivered signals including SIG_IGN.*/

might be slightly misleading. In most cases SIG_IGN signals won't make it
this far. The same is true of SIG_DFL signals whose default action is to
ignore. So one should be clear that trace_signal_deliver() is by no means
expected to get all SIG_IGN signals that are otherwise eligible for delivery.


Thanks,
Roland

2009-11-18 16:33:07

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip 0/3] tracepoint: Add signal events

Roland McGrath wrote:
> I would add the siginfo_t * argument to each of these tracepoints.

I see. Is it OK to record only si_errno and si_code? because si_signo
might be as same as sig.

> In 2/3 this comment:
>
> + /* Trace the actual delivered signals including SIG_IGN.*/
>
> might be slightly misleading. In most cases SIG_IGN signals won't make it
> this far. The same is true of SIG_DFL signals whose default action is to
> ignore. So one should be clear that trace_signal_deliver() is by no means
> expected to get all SIG_IGN signals that are otherwise eligible for delivery.

Ah, thank you for noticing. I'll remove the last words.

Thank you,

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-11-18 21:18:57

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH -tip 0/3] tracepoint: Add signal events

> Roland McGrath wrote:
> > I would add the siginfo_t * argument to each of these tracepoints.
>
> I see. Is it OK to record only si_errno and si_code? because si_signo
> might be as same as sig.

Yes, si_signo will match sig. Which siginfo_t fields are interesting to
record can vary by signal (i.e. by si_code). si_code and si_errno are
certainly the first things that come to mind as most useful. For SI_TIMER,
si_tid, si_overrun, and si_sigval are interesting. For others, si_pid and
si_uid are interesting.


Thanks,
Roland