2009-03-12 18:36:03

by Jason Baron

Subject: [Patch 1/2] 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.

Signed-off-by: Jason Baron <[email protected]>

---

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"
+};
+
/*
* 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;
}

2009-03-12 18:45:52

by Frederic Weisbecker

Subject: Re: [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array

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 <[email protected]>
>
> ---
>
> 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;
> }

2009-03-12 22:45:08

by Steven Rostedt

Subject: Re: [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array

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

2009-03-13 04:03:48

by Jason Baron

Subject: [tip:tracing/ftrace] tracing: tracepoints for softirq entry/exit - add softirq-to-name array

Commit-ID: 5d592b44b29a1d73e13d5c9e3426eed843bdc359
Gitweb: http://git.kernel.org/tip/5d592b44b29a1d73e13d5c9e3426eed843bdc359
Author: Jason Baron <[email protected]>
AuthorDate: Thu, 12 Mar 2009 14:33:36 -0400
Commit: Steven Rostedt <[email protected]>
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: <[email protected]>
Signed-off-by: Jason Baron <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>


---
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;
}

2009-03-14 02:53:51

by Mathieu Desnoyers

Subject: Re: [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array

* Jason Baron ([email protected]) 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 <[email protected]>
>
> ---
>
> 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

2009-03-16 19:00:45

by Jason Baron

Subject: Re: [Patch 1/2] tracepoints for softirq entry/exit - add softirq-to-name array

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 <[email protected]>


---

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;
}