* Re: [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array
2009-03-12 18:33 [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array Jason Baron
@ 2009-03-12 18:45 ` Frederic Weisbecker
2009-03-12 22:44 ` Steven Rostedt
2009-03-13 4:02 ` [tip:tracing/ftrace] tracing: " Jason Baron
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Frederic Weisbecker @ 2009-03-12 18:45 UTC (permalink / raw)
To: Jason Baron; +Cc: mingo, rostedt, linux-kernel, acme, fche, peterz, compudj
On Thu, Mar 12, 2009 at 02:33:36PM -0400, Jason Baron wrote:
>
> create a 'softirq_to_name' array, which is indexed by softirq #, so
> that we can easily convert between the softirq index # and its name, in
> order to get more meaningful output messages.
>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
>
> ---
>
> include/linux/interrupt.h | 5 +++++
> kernel/softirq.c | 9 ++++++++-
> 2 files changed, 13 insertions(+), 1 deletions(-)
>
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index e7bcfd7..da82ee4 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -258,6 +258,11 @@ enum
> NR_SOFTIRQS
> };
>
> +/* map softirq index to softirq name. update 'softirq_to_name' in
> + * kernel/softirq.c when adding a new softirq.
> + */
> +extern char *softirq_to_name[NR_SOFTIRQS];
> +
> /* softirq mask and active fields moved to irq_cpustat_t in
> * asm/hardirq.h to get better cache usage. KAO
> */
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 40ff3cf..ba1511f 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -53,6 +53,12 @@ static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp
>
> static DEFINE_PER_CPU(struct task_struct *, ksoftirqd);
>
> +char *softirq_to_name[NR_SOFTIRQS] = {
> + "HI_SOFTIRQ", "TIMER_SOFTIRQ", "NET_TX_SOFTIRQ", "NET_RX_SOFTIRQ",
> + "BLOCK_SOFTIRQ", "TASKLET_SOFTIRQ", "SCHED_SOFTIRQ", "HRTIMER_SOFTIRQ",
> + "RCU_SOFTIRQ"
> +};
Hi,
May be you could abuse open_softirq() to append dynamically these entries:
char *softirq_to_name[NR_SOFTIRQS];
#define open_softirq(nr, func) \
softirq_to_name[nr] = __stringify(nr); \
__open_softirq(nr, func);
So that it's a bit more scalable.
Frederic.
> +
> /*
> * we cannot loop indefinitely here to avoid userspace starvation,
> * but we also don't want to introduce a worst case 1/HZ latency
> @@ -209,9 +215,10 @@ restart:
> h->action(h);
>
> if (unlikely(prev_count != preempt_count())) {
> - printk(KERN_ERR "huh, entered softirq %td %p"
> + printk(KERN_ERR "huh, entered softirq %td %s %p"
> "with preempt_count %08x,"
> " exited with %08x?\n", h - softirq_vec,
> + softirq_to_name[h - softirq_vec],
> h->action, prev_count, preempt_count());
> preempt_count() = prev_count;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array
2009-03-12 18:45 ` Frederic Weisbecker
@ 2009-03-12 22:44 ` Steven Rostedt
0 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2009-03-12 22:44 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Jason Baron, mingo, linux-kernel, acme, fche, peterz, compudj
On Thu, 12 Mar 2009, Frederic Weisbecker wrote:
> > @@ -53,6 +53,12 @@ static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp
> >
> > static DEFINE_PER_CPU(struct task_struct *, ksoftirqd);
> >
> > +char *softirq_to_name[NR_SOFTIRQS] = {
> > + "HI_SOFTIRQ", "TIMER_SOFTIRQ", "NET_TX_SOFTIRQ", "NET_RX_SOFTIRQ",
> > + "BLOCK_SOFTIRQ", "TASKLET_SOFTIRQ", "SCHED_SOFTIRQ", "HRTIMER_SOFTIRQ",
> > + "RCU_SOFTIRQ"
> > +};
>
>
> Hi,
>
> May be you could abuse open_softirq() to append dynamically these entries:
>
> char *softirq_to_name[NR_SOFTIRQS];
>
> #define open_softirq(nr, func) \
> softirq_to_name[nr] = __stringify(nr); \
> __open_softirq(nr, func);
>
> So that it's a bit more scalable.
>
Softirqs are pretty much set. If anything, they may even disappear (if the
threaded interrupts take off). So hardcoding it should not be an issue.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:tracing/ftrace] tracing: tracepoints for softirq entry/exit - add softirq-to-name array
2009-03-12 18:33 [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array Jason Baron
2009-03-12 18:45 ` Frederic Weisbecker
@ 2009-03-13 4:02 ` Jason Baron
2009-03-14 2:53 ` [Patch 1/2] " Mathieu Desnoyers
2009-03-16 18:58 ` Jason Baron
3 siblings, 0 replies; 6+ messages in thread
From: Jason Baron @ 2009-03-13 4:02 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, srostedt, jbaron, tglx
Commit-ID: 5d592b44b29a1d73e13d5c9e3426eed843bdc359
Gitweb: http://git.kernel.org/tip/5d592b44b29a1d73e13d5c9e3426eed843bdc359
Author: Jason Baron <jbaron@redhat.com>
AuthorDate: Thu, 12 Mar 2009 14:33:36 -0400
Commit: Steven Rostedt <srostedt@redhat.com>
CommitDate: Thu, 12 Mar 2009 21:15:02 -0400
tracing: tracepoints for softirq entry/exit - add softirq-to-name array
Create a 'softirq_to_name' array, which is indexed by softirq #, so
that we can easily convert between the softirq index # and its name, in
order to get more meaningful output messages.
LKML-Reference: <20090312183336.GB3352@redhat.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
include/linux/interrupt.h | 5 +++++
kernel/softirq.c | 9 ++++++++-
2 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 472f117..9b7e9d7 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -258,6 +258,11 @@ enum
NR_SOFTIRQS
};
+/* map softirq index to softirq name. update 'softirq_to_name' in
+ * kernel/softirq.c when adding a new softirq.
+ */
+extern char *softirq_to_name[NR_SOFTIRQS];
+
/* softirq mask and active fields moved to irq_cpustat_t in
* asm/hardirq.h to get better cache usage. KAO
*/
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 7571bcb..9f90fdc 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -53,6 +53,12 @@ static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp
static DEFINE_PER_CPU(struct task_struct *, ksoftirqd);
+char *softirq_to_name[NR_SOFTIRQS] = {
+ "HI_SOFTIRQ", "TIMER_SOFTIRQ", "NET_TX_SOFTIRQ", "NET_RX_SOFTIRQ",
+ "BLOCK_SOFTIRQ", "TASKLET_SOFTIRQ", "SCHED_SOFTIRQ", "HRTIMER_SOFTIRQ",
+ "RCU_SOFTIRQ"
+};
+
/*
* we cannot loop indefinitely here to avoid userspace starvation,
* but we also don't want to introduce a worst case 1/HZ latency
@@ -209,9 +215,10 @@ restart:
h->action(h);
if (unlikely(prev_count != preempt_count())) {
- printk(KERN_ERR "huh, entered softirq %td %p"
+ printk(KERN_ERR "huh, entered softirq %td %s %p"
"with preempt_count %08x,"
" exited with %08x?\n", h - softirq_vec,
+ softirq_to_name[h - softirq_vec],
h->action, prev_count, preempt_count());
preempt_count() = prev_count;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array
2009-03-12 18:33 [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array Jason Baron
2009-03-12 18:45 ` Frederic Weisbecker
2009-03-13 4:02 ` [tip:tracing/ftrace] tracing: " Jason Baron
@ 2009-03-14 2:53 ` Mathieu Desnoyers
2009-03-16 18:58 ` Jason Baron
3 siblings, 0 replies; 6+ messages in thread
From: Mathieu Desnoyers @ 2009-03-14 2:53 UTC (permalink / raw)
To: Jason Baron; +Cc: mingo, rostedt, linux-kernel, acme, fweisbec, fche, peterz
* Jason Baron (jbaron@redhat.com) wrote:
>
> create a 'softirq_to_name' array, which is indexed by softirq #, so
> that we can easily convert between the softirq index # and its name, in
> order to get more meaningful output messages.
>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
>
> ---
>
> include/linux/interrupt.h | 5 +++++
> kernel/softirq.c | 9 ++++++++-
> 2 files changed, 13 insertions(+), 1 deletions(-)
>
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index e7bcfd7..da82ee4 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -258,6 +258,11 @@ enum
> NR_SOFTIRQS
> };
>
> +/* map softirq index to softirq name. update 'softirq_to_name' in
> + * kernel/softirq.c when adding a new softirq.
> + */
> +extern char *softirq_to_name[NR_SOFTIRQS];
> +
> /* softirq mask and active fields moved to irq_cpustat_t in
> * asm/hardirq.h to get better cache usage. KAO
> */
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 40ff3cf..ba1511f 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -53,6 +53,12 @@ static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp
>
> static DEFINE_PER_CPU(struct task_struct *, ksoftirqd);
>
> +char *softirq_to_name[NR_SOFTIRQS] = {
> + "HI_SOFTIRQ", "TIMER_SOFTIRQ", "NET_TX_SOFTIRQ", "NET_RX_SOFTIRQ",
> + "BLOCK_SOFTIRQ", "TASKLET_SOFTIRQ", "SCHED_SOFTIRQ", "HRTIMER_SOFTIRQ",
> + "RCU_SOFTIRQ"
> +};
> +
Small nit :
this should be a const char *.
Besides that it looks good, and very useful to a tracer. We just have to
make sure this table can eventually be saved into a trace.
Mathieu
> /*
> * we cannot loop indefinitely here to avoid userspace starvation,
> * but we also don't want to introduce a worst case 1/HZ latency
> @@ -209,9 +215,10 @@ restart:
> h->action(h);
>
> if (unlikely(prev_count != preempt_count())) {
> - printk(KERN_ERR "huh, entered softirq %td %p"
> + printk(KERN_ERR "huh, entered softirq %td %s %p"
> "with preempt_count %08x,"
> " exited with %08x?\n", h - softirq_vec,
> + softirq_to_name[h - softirq_vec],
> h->action, prev_count, preempt_count());
> preempt_count() = prev_count;
> }
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array
2009-03-12 18:33 [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array Jason Baron
` (2 preceding siblings ...)
2009-03-14 2:53 ` [Patch 1/2] " Mathieu Desnoyers
@ 2009-03-16 18:58 ` Jason Baron
3 siblings, 0 replies; 6+ messages in thread
From: Jason Baron @ 2009-03-16 18:58 UTC (permalink / raw)
To: mingo, rostedt; +Cc: linux-kernel, acme, fweisbec, fche, peterz, compudj
hi,
I've update both patches to incorporate changes that were suggested -
I've shortened the softirq string name and made it 'const'. Below is a
re-spun patch 1/2.
thanks,
-Jason
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
include/linux/interrupt.h | 6 ++++++
kernel/softirq.c | 8 +++++++-
2 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index e7bcfd7..dd3ff3f 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -258,6 +258,12 @@ enum
NR_SOFTIRQS
};
+/* map softirq index to softirq name. update 'softirq_to_name' in
+ * kernel/softirq.c when adding a new softirq.
+ */
+#define MAX_SOFTIRQ_NAME_LEN 10
+extern const char softirq_to_name[NR_SOFTIRQS][MAX_SOFTIRQ_NAME_LEN];
+
/* softirq mask and active fields moved to irq_cpustat_t in
* asm/hardirq.h to get better cache usage. KAO
*/
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 40ff3cf..8f3ae57 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -53,6 +53,11 @@ static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp
static DEFINE_PER_CPU(struct task_struct *, ksoftirqd);
+const char softirq_to_name[NR_SOFTIRQS][MAX_SOFTIRQ_NAME_LEN] = {
+ "HI", "TIMER", "NET_TX", "NET_RX", "BLOCK", "TASKLET", "SCHED",
+ "HRTIMER", "RCU"
+};
+
/*
* we cannot loop indefinitely here to avoid userspace starvation,
* but we also don't want to introduce a worst case 1/HZ latency
@@ -209,9 +214,10 @@ restart:
h->action(h);
if (unlikely(prev_count != preempt_count())) {
- printk(KERN_ERR "huh, entered softirq %td %p"
+ printk(KERN_ERR "huh, entered softirq %td %s %p"
"with preempt_count %08x,"
" exited with %08x?\n", h - softirq_vec,
+ softirq_to_name[h - softirq_vec],
h->action, prev_count, preempt_count());
preempt_count() = prev_count;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread