2008-11-22 04:22:42

by Keika Kobayashi

[permalink] [raw]
Subject: [PATCH 0/3 v2] softirq: Statistics for softirq

This is v2 of statistics for softirq patch series.
Thanks to Andrew for comments!
These patches are fixed about the following.

Against: next-20081121

Change Log v1 -> v2
[2/3] ... proc: Export statistics for softirq to /proc
o Change from for_each_online_cpu() to for_each_possible_cpu()
in show_softirqs().
o Make "proc_softirqs_operations" const.
o Don't introduce for_each_softirq_nr() for readability.


2008-11-22 04:25:53

by Keika Kobayashi

[permalink] [raw]
Subject: [PATCH 1/3 v2] softirq: Introduce statistics for softirq

Statistics for softirq doesn't exist.
It will be helpful like statistics for interrupts.
This patch introduces counting the number of softirq,
which will be exported in /proc/softirqs.

When softirq handler consumes much CPU time,
/proc/stat is like the following.

$ while :; do cat /proc/stat | head -n1 ; sleep 10 ; done
cpu 88 0 408 739665 583 28 2 0 0
cpu 450 0 1090 740970 594 28 1294 0 0
^^^^
softirq

In such a situation,
/proc/softirqs shows us which softirq handler is invoked.
We can see the increase rate of softirqs.

<before>
$ cat /proc/softirqs
CPU0 CPU1 CPU2 CPU3
HI 0 0 0 0
TIMER 462850 462805 462782 462718
NET_TX 0 0 0 365
NET_RX 2472 2 2 40
BLOCK 0 0 381 1164
TASKLET 0 0 0 224
SCHED 462654 462689 462698 462427
RCU 3046 2423 3367 3173

<after>
$ cat /proc/softirqs
CPU0 CPU1 CPU2 CPU3
HI 0 0 0 0
TIMER 463361 465077 465056 464991
NET_TX 53 0 1 365
NET_RX 3757 2 2 40
BLOCK 0 0 398 1170
TASKLET 0 0 0 224
SCHED 463074 464318 464612 463330
RCU 3505 2948 3947 3673

When CPU TIME of softirq is high,
the rates of increase is the following.
TIMER : 220/sec : CPU1-3
NET_TX : 5/sec : CPU0
NET_RX : 120/sec : CPU0
SCHED : 40-200/sec : all CPU
RCU : 45-58/sec : all CPU

The rates of increase in an idle mode is the following.
TIMER : 250/sec
SCHED : 250/sec
RCU : 2/sec

It seems many softirqs for receiving packets and rcu are invoked.
This gives us help for checking system.

Signed-off-by: Keika Kobayashi <[email protected]>
Reviewed-by: Hiroshi Shimamoto <[email protected]>
---
include/linux/kernel_stat.h | 12 ++++++++++++
kernel/softirq.c | 1 +
2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/include/linux/kernel_stat.h b/include/linux/kernel_stat.h
index 4a145ca..57c1643 100644
--- a/include/linux/kernel_stat.h
+++ b/include/linux/kernel_stat.h
@@ -5,6 +5,7 @@
#include <linux/threads.h>
#include <linux/percpu.h>
#include <linux/cpumask.h>
+#include <linux/interrupt.h>
#include <asm/irq.h>
#include <asm/cputime.h>

@@ -29,6 +30,7 @@ struct cpu_usage_stat {
struct kernel_stat {
struct cpu_usage_stat cpustat;
unsigned int irqs[NR_IRQS];
+ unsigned int softirqs[NR_SOFTIRQS];
};

DECLARE_PER_CPU(struct kernel_stat, kstat);
@@ -52,6 +54,16 @@ static inline unsigned int kstat_irqs_cpu(unsigned int irq, int cpu)
return kstat_cpu(cpu).irqs[irq];
}

+static inline void kstat_incr_softirqs_this_cpu(unsigned int irq)
+{
+ kstat_this_cpu.softirqs[irq]++;
+}
+
+static inline unsigned int kstat_softirqs_cpu(unsigned int irq, int cpu)
+{
+ return kstat_cpu(cpu).softirqs[irq];
+}
+
/*
* Number of interrupts per specific IRQ source, since bootup
*/
diff --git a/kernel/softirq.c b/kernel/softirq.c
index e7c69a7..088e179 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -208,6 +208,7 @@ restart:
do {
if (pending & 1) {
int prev_count = preempt_count();
+ kstat_incr_softirqs_this_cpu(h - softirq_vec);

h->action(h);

--
1.5.0.6

2008-11-22 04:27:20

by Keika Kobayashi

[permalink] [raw]
Subject: [PATCH 2/3 v2] proc: Export statistics for softirq to /proc

Export statistics for softirq in /proc/softirqs and /proc/stat.

1. /proc/softirqs
Implement /proc/softirqs which shows the number of softirq
for each CPU like /proc/interrupts.

2. /proc/stat
Add the "softirq" line to /proc/stat.
This line shows the number of softirq for all cpu.
The first column is the total of all softirqs and
each subsequent column is the total for particular softirq.

Signed-off-by: Keika Kobayashi <[email protected]>
Reviewed-by: Hiroshi Shimamoto <[email protected]>
---
fs/proc/Makefile | 1 +
fs/proc/softirqs.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
fs/proc/stat.c | 17 +++++++++++++++
3 files changed, 75 insertions(+), 0 deletions(-)
create mode 100644 fs/proc/softirqs.c

diff --git a/fs/proc/Makefile b/fs/proc/Makefile
index 63d9651..11a7b5c 100644
--- a/fs/proc/Makefile
+++ b/fs/proc/Makefile
@@ -18,6 +18,7 @@ proc-y += meminfo.o
proc-y += stat.o
proc-y += uptime.o
proc-y += version.o
+proc-y += softirqs.o
proc-$(CONFIG_PROC_SYSCTL) += proc_sysctl.o
proc-$(CONFIG_NET) += proc_net.o
proc-$(CONFIG_PROC_KCORE) += kcore.o
diff --git a/fs/proc/softirqs.c b/fs/proc/softirqs.c
new file mode 100644
index 0000000..543f9d9
--- /dev/null
+++ b/fs/proc/softirqs.c
@@ -0,0 +1,57 @@
+#include <linux/init.h>
+#include <linux/kernel_stat.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+
+static const char *desc_array[] = {
+ "HI",
+ "TIMER",
+ "NET_TX",
+ "NET_RX",
+ "BLOCK",
+ "TASKLET",
+ "SCHED",
+#ifdef CONFIG_HIGH_RES_TIMERS
+ "HRTIMER",
+#endif
+ "RCU"};
+
+/*
+ * /proc/softirqs ... display the number of softirqs
+ */
+static int show_softirqs(struct seq_file *p, void *v)
+{
+ int i, j;
+
+ seq_printf(p, " ");
+ for_each_possible_cpu(i)
+ seq_printf(p, "CPU%-8d", i);
+ seq_printf(p, "\n");
+
+ for (i = 0; i < NR_SOFTIRQS; i++) {
+ seq_printf(p, "%-9s", desc_array[i]);
+ for_each_possible_cpu(j)
+ seq_printf(p, " %10u", kstat_softirqs_cpu(i, j));
+ seq_printf(p, "\n");
+ }
+ return 0;
+}
+
+static int softirqs_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, show_softirqs, NULL);
+}
+
+static const struct file_operations proc_softirqs_operations = {
+ .open = softirqs_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static int __init proc_softirqs_init(void)
+{
+ proc_create("softirqs", 0, NULL, &proc_softirqs_operations);
+ return 0;
+}
+module_init(proc_softirqs_init);
diff --git a/fs/proc/stat.c b/fs/proc/stat.c
index 81904f0..74ac85a 100644
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -25,6 +25,7 @@ static int show_stat(struct seq_file *p, void *v)
cputime64_t user, nice, system, idle, iowait, irq, softirq, steal;
cputime64_t guest;
u64 sum = 0;
+ u64 sum_softirq = 0;
struct timespec boottime;
unsigned int per_irq_sum;

@@ -49,6 +50,10 @@ static int show_stat(struct seq_file *p, void *v)
sum += kstat_irqs_cpu(j, i);

sum += arch_irq_stat_cpu(i);
+
+ for (j = 0; j < NR_SOFTIRQS; j++)
+ sum_softirq += kstat_softirqs_cpu(j, i);
+
}
sum += arch_irq_stat();

@@ -111,6 +116,18 @@ static int show_stat(struct seq_file *p, void *v)
nr_running(),
nr_iowait());

+ seq_printf(p, "softirq %llu", (unsigned long long)sum_softirq);
+
+ for (i = 0; i < NR_SOFTIRQS; i++) {
+ per_irq_sum = 0;
+
+ for_each_possible_cpu(j)
+ per_irq_sum += kstat_softirqs_cpu(i, j);
+
+ seq_printf(p, " %u", per_irq_sum);
+ }
+ seq_printf(p, "\n");
+
return 0;
}

--
1.5.0.6

2008-11-22 04:28:26

by Keika Kobayashi

[permalink] [raw]
Subject: [PATCH 3/3 v2] proc: Update document for /proc/softirqs and /proc/stat

/proc/softirqs and /proc/stat have been changed.
This patch updates proc.txt for these usage.

Signed-off-by: Keika Kobayashi <[email protected]>
Reviewed-by: Hiroshi Shimamoto <[email protected]>
---
Documentation/filesystems/proc.txt | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index bcceb99..ef1eeae 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -288,6 +288,7 @@ Table 1-4: Kernel info in /proc
rtc Real time clock
scsi SCSI info (see text)
slabinfo Slab pool info
+ softirqs softirq usage
stat Overall statistics
swaps Swap space utilization
sys See chapter 2
@@ -602,6 +603,24 @@ on the kind of area :
0xffffffffa0017000-0xffffffffa0022000 45056 sys_init_module+0xc27/0x1d00 ...
pages=10 vmalloc N0=10

+..............................................................................
+
+softirqs:
+
+Provides counts of softirq handlers serviced since boot time, for each cpu.
+
+> cat /proc/softirqs
+ CPU0 CPU1 CPU2 CPU3
+HI 0 0 0 0
+TIMER 27166 27120 27097 27034
+NET_TX 0 0 0 17
+NET_RX 42 0 0 39
+BLOCK 0 0 107 1121
+TASKLET 0 0 0 290
+SCHED 27035 26983 26971 26746
+RCU 1678 1769 2178 2250
+
+
1.3 IDE devices in /proc/ide
----------------------------

@@ -888,6 +907,7 @@ since the system first booted. For a quick look, simply cat the file:
processes 2915
procs_running 1
procs_blocked 0
+ softirq 183433 0 21755 12 39 1137 231 21459 2263

The very first "cpu" line aggregates the numbers in all of the other "cpuN"
lines. These numbers identify the amount of time the CPU has spent performing
@@ -923,6 +943,11 @@ CPUs.
The "procs_blocked" line gives the number of processes currently blocked,
waiting for I/O to complete.

+The "softirq" line gives counts of softirqs serviced since boot time, for each
+of the possible system softirqs. The first column is the total of all
+softirqs serviced; each subsequent column is the total for that particular
+softirq.
+

1.9 Ext4 file system parameters
------------------------------
--
1.5.0.6

2008-11-22 05:36:48

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 1/3 v2] softirq: Introduce statistics for softirq

> Statistics for softirq doesn't exist.
> It will be helpful like statistics for interrupts.
> This patch introduces counting the number of softirq,
> which will be exported in /proc/softirqs.
>
> When softirq handler consumes much CPU time,
> /proc/stat is like the following.
>
> $ while :; do cat /proc/stat | head -n1 ; sleep 10 ; done
> cpu 88 0 408 739665 583 28 2 0 0
> cpu 450 0 1090 740970 594 28 1294 0 0
> ^^^^
> softirq
>
> In such a situation,
> /proc/softirqs shows us which softirq handler is invoked.
> We can see the increase rate of softirqs.
>
> <before>
> $ cat /proc/softirqs
> CPU0 CPU1 CPU2 CPU3
> HI 0 0 0 0
> TIMER 462850 462805 462782 462718
> NET_TX 0 0 0 365
> NET_RX 2472 2 2 40
> BLOCK 0 0 381 1164
> TASKLET 0 0 0 224
> SCHED 462654 462689 462698 462427
> RCU 3046 2423 3367 3173
>
> <after>
> $ cat /proc/softirqs
> CPU0 CPU1 CPU2 CPU3
> HI 0 0 0 0
> TIMER 463361 465077 465056 464991
> NET_TX 53 0 1 365
> NET_RX 3757 2 2 40
> BLOCK 0 0 398 1170
> TASKLET 0 0 0 224
> SCHED 463074 464318 464612 463330
> RCU 3505 2948 3947 3673

Maybe, this printing format don't works well on >100 CPUS.

Do you have any experience of trouble-shooting by this patch?
Actually, I don't understand this patch usuful working situation.

Honestly, I think HI and TASKLET are not beneficialness.
end user can't know what tasklet spent time.
TIMER also are not so much useful because it always have the same rate.

So.. guessing...

ultra band-width networking problem?



>
> When CPU TIME of softirq is high,
> the rates of increase is the following.
> TIMER : 220/sec : CPU1-3
> NET_TX : 5/sec : CPU0
> NET_RX : 120/sec : CPU0
> SCHED : 40-200/sec : all CPU
> RCU : 45-58/sec : all CPU
>
> The rates of increase in an idle mode is the following.
> TIMER : 250/sec
> SCHED : 250/sec
> RCU : 2/sec
>
> It seems many softirqs for receiving packets and rcu are invoked.
> This gives us help for checking system.
>
> Signed-off-by: Keika Kobayashi <[email protected]>
> Reviewed-by: Hiroshi Shimamoto <[email protected]>
> ---
> include/linux/kernel_stat.h | 12 ++++++++++++
> kernel/softirq.c | 1 +
> 2 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/kernel_stat.h b/include/linux/kernel_stat.h
> index 4a145ca..57c1643 100644
> --- a/include/linux/kernel_stat.h
> +++ b/include/linux/kernel_stat.h
> @@ -5,6 +5,7 @@
> #include <linux/threads.h>
> #include <linux/percpu.h>
> #include <linux/cpumask.h>
> +#include <linux/interrupt.h>
> #include <asm/irq.h>
> #include <asm/cputime.h>
>
> @@ -29,6 +30,7 @@ struct cpu_usage_stat {
> struct kernel_stat {
> struct cpu_usage_stat cpustat;
> unsigned int irqs[NR_IRQS];
> + unsigned int softirqs[NR_SOFTIRQS];
> };
>
> DECLARE_PER_CPU(struct kernel_stat, kstat);
> @@ -52,6 +54,16 @@ static inline unsigned int kstat_irqs_cpu(unsigned int irq, int cpu)
> return kstat_cpu(cpu).irqs[irq];
> }
>
> +static inline void kstat_incr_softirqs_this_cpu(unsigned int irq)
> +{
> + kstat_this_cpu.softirqs[irq]++;
> +}
> +
> +static inline unsigned int kstat_softirqs_cpu(unsigned int irq, int cpu)
> +{
> + return kstat_cpu(cpu).softirqs[irq];
> +}
> +
> /*
> * Number of interrupts per specific IRQ source, since bootup
> */
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index e7c69a7..088e179 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -208,6 +208,7 @@ restart:
> do {
> if (pending & 1) {
> int prev_count = preempt_count();
> + kstat_incr_softirqs_this_cpu(h - softirq_vec);
>
> h->action(h);

you should explain this patch don't decrease system performance.


2008-11-22 11:37:17

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] proc: Export statistics for softirq to /proc

Keika Kobayashi a ?crit :
> Export statistics for softirq in /proc/softirqs and /proc/stat.
>
> 1. /proc/softirqs
> Implement /proc/softirqs which shows the number of softirq
> for each CPU like /proc/interrupts.
>
> 2. /proc/stat
> Add the "softirq" line to /proc/stat.
> This line shows the number of softirq for all cpu.
> The first column is the total of all softirqs and
> each subsequent column is the total for particular softirq.
>
> Signed-off-by: Keika Kobayashi <[email protected]>
> Reviewed-by: Hiroshi Shimamoto <[email protected]>
> ---
> fs/proc/Makefile | 1 +
> fs/proc/softirqs.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/proc/stat.c | 17 +++++++++++++++
> 3 files changed, 75 insertions(+), 0 deletions(-)
> create mode 100644 fs/proc/softirqs.c
>
> diff --git a/fs/proc/Makefile b/fs/proc/Makefile
> index 63d9651..11a7b5c 100644
> --- a/fs/proc/Makefile
> +++ b/fs/proc/Makefile
> @@ -18,6 +18,7 @@ proc-y += meminfo.o
> proc-y += stat.o
> proc-y += uptime.o
> proc-y += version.o
> +proc-y += softirqs.o
> proc-$(CONFIG_PROC_SYSCTL) += proc_sysctl.o
> proc-$(CONFIG_NET) += proc_net.o
> proc-$(CONFIG_PROC_KCORE) += kcore.o
> diff --git a/fs/proc/softirqs.c b/fs/proc/softirqs.c
> new file mode 100644
> index 0000000..543f9d9
> --- /dev/null
> +++ b/fs/proc/softirqs.c
> @@ -0,0 +1,57 @@
> +#include <linux/init.h>
> +#include <linux/kernel_stat.h>
> +#include <linux/proc_fs.h>
> +#include <linux/seq_file.h>
> +
> +static const char *desc_array[] = {
> + "HI",
> + "TIMER",
> + "NET_TX",
> + "NET_RX",
> + "BLOCK",
> + "TASKLET",
> + "SCHED",
> +#ifdef CONFIG_HIGH_RES_TIMERS
> + "HRTIMER",
> +#endif
> + "RCU"};
> +


You could use C99 initializers here

[HI_SOFTIRQ] = "HI",
[TIMER_SOFTIRQ] = "TIMER",
...

2008-11-24 23:46:22

by Keika Kobayashi

[permalink] [raw]
Subject: Re: [PATCH 1/3 v2] softirq: Introduce statistics for softirq

On Sat, 22 Nov 2008 14:36:34 +0900 (JST)
KOSAKI Motohiro <[email protected]> wrote:

> > Statistics for softirq doesn't exist.
> > It will be helpful like statistics for interrupts.
> > This patch introduces counting the number of softirq,
> > which will be exported in /proc/softirqs.
> >
> > When softirq handler consumes much CPU time,
> > /proc/stat is like the following.
> >
> > $ while :; do cat /proc/stat | head -n1 ; sleep 10 ; done
> > cpu 88 0 408 739665 583 28 2 0 0
> > cpu 450 0 1090 740970 594 28 1294 0 0
> > ^^^^
> > softirq
> >
> > In such a situation,
> > /proc/softirqs shows us which softirq handler is invoked.
> > We can see the increase rate of softirqs.
> >
> > <before>
> > $ cat /proc/softirqs
> > CPU0 CPU1 CPU2 CPU3
> > HI 0 0 0 0
> > TIMER 462850 462805 462782 462718
> > NET_TX 0 0 0 365
> > NET_RX 2472 2 2 40
> > BLOCK 0 0 381 1164
> > TASKLET 0 0 0 224
> > SCHED 462654 462689 462698 462427
> > RCU 3046 2423 3367 3173
> >
> > <after>
> > $ cat /proc/softirqs
> > CPU0 CPU1 CPU2 CPU3
> > HI 0 0 0 0
> > TIMER 463361 465077 465056 464991
> > NET_TX 53 0 1 365
> > NET_RX 3757 2 2 40
> > BLOCK 0 0 398 1170
> > TASKLET 0 0 0 224
> > SCHED 463074 464318 464612 463330
> > RCU 3505 2948 3947 3673
>
> Maybe, this printing format don't works well on >100 CPUS.
>
> Do you have any experience of trouble-shooting by this patch?
> Actually, I don't understand this patch usuful working situation.

When reported that CPU TIME of softirq went up,
we had to investigate the cause.
In that case, kind of network looked suspicious.
We added such a function to confirm.

Original patch shows ticks of softirqs.
But /proc/interrups have counts of interrupts.
So we changed from ticks to counts.

> Honestly, I think HI and TASKLET are not beneficialness.
> end user can't know what tasklet spent time.
> TIMER also are not so much useful because it always have the same rate.
>
> So.. guessing...
>
> ultra band-width networking problem?
>
>
<snip>
>
> > /*
> > * Number of interrupts per specific IRQ source, since bootup
> > */
> > diff --git a/kernel/softirq.c b/kernel/softirq.c
> > index e7c69a7..088e179 100644
> > --- a/kernel/softirq.c
> > +++ b/kernel/softirq.c
> > @@ -208,6 +208,7 @@ restart:
> > do {
> > if (pending & 1) {
> > int prev_count = preempt_count();
> > + kstat_incr_softirqs_this_cpu(h - softirq_vec);
> >
> > h->action(h);
>
> you should explain this patch don't decrease system performance.

In this pach, only kstat_incr_softirqs_this_cpu() may
influence system performance.
But this just increments one variable.
I don't think we can detect the decrease in the performance.

2008-11-26 10:10:41

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] proc: Export statistics for softirq to /proc

nit

> @@ -49,6 +50,10 @@ static int show_stat(struct seq_file *p, void *v)
> sum += kstat_irqs_cpu(j, i);
>
> sum += arch_irq_stat_cpu(i);
> +
> + for (j = 0; j < NR_SOFTIRQS; j++)
> + sum_softirq += kstat_softirqs_cpu(j, i);
> +
> }

You can calcurate per_irq_sum here.
Typically, # of possible cpu are very big.

So, I don't like unnecessary twrice looping.


> sum += arch_irq_stat();
>
> @@ -111,6 +116,18 @@ static int show_stat(struct seq_file *p, void *v)
> nr_running(),
> nr_iowait());
>
> + seq_printf(p, "softirq %llu", (unsigned long long)sum_softirq);
> +
> + for (i = 0; i < NR_SOFTIRQS; i++) {
> + per_irq_sum = 0;
> +
> + for_each_possible_cpu(j)
> + per_irq_sum += kstat_softirqs_cpu(i, j);
> +
> + seq_printf(p, " %u", per_irq_sum);
> + }
> + seq_printf(p, "\n");
> +
> return 0;
> }

2008-11-26 10:11:19

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 3/3 v2] proc: Update document for /proc/softirqs and /proc/stat

> /proc/softirqs and /proc/stat have been changed.
> This patch updates proc.txt for these usage.
>
> Signed-off-by: Keika Kobayashi <[email protected]>
> Reviewed-by: Hiroshi Shimamoto <[email protected]>

looks good to me.
Reviewed-by: KOSAKI Motohiro <[email protected]>


2008-11-26 10:15:19

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 1/3 v2] softirq: Introduce statistics for softirq

> On Sat, 22 Nov 2008 14:36:34 +0900 (JST)
> KOSAKI Motohiro <[email protected]> wrote:
>
> > > Statistics for softirq doesn't exist.
> > > It will be helpful like statistics for interrupts.
> > > This patch introduces counting the number of softirq,
> > > which will be exported in /proc/softirqs.
> > >
> > > When softirq handler consumes much CPU time,
> > > /proc/stat is like the following.
> > >
> > > $ while :; do cat /proc/stat | head -n1 ; sleep 10 ; done
> > > cpu 88 0 408 739665 583 28 2 0 0
> > > cpu 450 0 1090 740970 594 28 1294 0 0
> > > ^^^^
> > > softirq
> > >
> > > In such a situation,
> > > /proc/softirqs shows us which softirq handler is invoked.
> > > We can see the increase rate of softirqs.
> > >
> > > <before>
> > > $ cat /proc/softirqs
> > > CPU0 CPU1 CPU2 CPU3
> > > HI 0 0 0 0
> > > TIMER 462850 462805 462782 462718
> > > NET_TX 0 0 0 365
> > > NET_RX 2472 2 2 40
> > > BLOCK 0 0 381 1164
> > > TASKLET 0 0 0 224
> > > SCHED 462654 462689 462698 462427
> > > RCU 3046 2423 3367 3173
> > >
> > > <after>
> > > $ cat /proc/softirqs
> > > CPU0 CPU1 CPU2 CPU3
> > > HI 0 0 0 0
> > > TIMER 463361 465077 465056 464991
> > > NET_TX 53 0 1 365
> > > NET_RX 3757 2 2 40
> > > BLOCK 0 0 398 1170
> > > TASKLET 0 0 0 224
> > > SCHED 463074 464318 464612 463330
> > > RCU 3505 2948 3947 3673
> >
> > Maybe, this printing format don't works well on >100 CPUS.
> >
> > Do you have any experience of trouble-shooting by this patch?
> > Actually, I don't understand this patch usuful working situation.
>
> When reported that CPU TIME of softirq went up,
> we had to investigate the cause.
> In that case, kind of network looked suspicious.
> We added such a function to confirm.
>
> Original patch shows ticks of softirqs.
> But /proc/interrups have counts of interrupts.
> So we changed from ticks to counts.

ok. it seems reasonable reason.
and this patch looks good to me.

Reviewed-by: KOSAKI Motohiro <[email protected]>


>
> > Honestly, I think HI and TASKLET are not beneficialness.
> > end user can't know what tasklet spent time.
> > TIMER also are not so much useful because it always have the same rate.
> >
> > So.. guessing...
> >
> > ultra band-width networking problem?

2009-01-09 22:17:10

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] proc: Export statistics for softirq to /proc

On Wed, 26 Nov 2008 19:10:28 +0900 (JST)
KOSAKI Motohiro <[email protected]> wrote:

> nit
>
> > @@ -49,6 +50,10 @@ static int show_stat(struct seq_file *p, void *v)
> > sum += kstat_irqs_cpu(j, i);
> >
> > sum += arch_irq_stat_cpu(i);
> > +
> > + for (j = 0; j < NR_SOFTIRQS; j++)
> > + sum_softirq += kstat_softirqs_cpu(j, i);
> > +
> > }
>
> You can calcurate per_irq_sum here.
> Typically, # of possible cpu are very big.
>
> So, I don't like unnecessary twrice looping.

I was about to send these patches to Linus, but it seems that this
optmisation hasn't been addressed?

2009-01-10 18:22:35

by Keika Kobayashi

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] proc: Export statistics for softirq to /proc

2009/1/9 Andrew Morton <[email protected]>:
> On Wed, 26 Nov 2008 19:10:28 +0900 (JST)
> KOSAKI Motohiro <[email protected]> wrote:
>
>> nit
>>
>> > @@ -49,6 +50,10 @@ static int show_stat(struct seq_file *p, void *v)
>> > sum += kstat_irqs_cpu(j, i);
>> >
>> > sum += arch_irq_stat_cpu(i);
>> > +
>> > + for (j = 0; j < NR_SOFTIRQS; j++)
>> > + sum_softirq += kstat_softirqs_cpu(j, i);
>> > +
>> > }
>>
>> You can calcurate per_irq_sum here.
>> Typically, # of possible cpu are very big.
>>
>> So, I don't like unnecessary twrice looping.
>
> I was about to send these patches to Linus, but it seems that this
> optmisation hasn't been addressed?

Thanks for your notice, Andrew.
Of course, thanks for your advice, Kosaki-san.
I didn't check carefully about this point...

Now I am on a business trip.
So, I'll post the fix next week.

// Keika Kobayashi

2009-01-10 19:44:48

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] proc: Export statistics for softirq to /proc

>>> You can calcurate per_irq_sum here.
>>> Typically, # of possible cpu are very big.
>>>
>>> So, I don't like unnecessary twrice looping.
>>
>> I was about to send these patches to Linus, but it seems that this
>> optmisation hasn't been addressed?
>
> Thanks for your notice, Andrew.
> Of course, thanks for your advice, Kosaki-san.
> I didn't check carefully about this point...
>
> Now I am on a business trip.
> So, I'll post the fix next week.

No problem.
I've made it three hour ago. I'll post soon.

2009-01-12 11:48:10

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] proc: Export statistics for softirq to /proc

> >>> You can calcurate per_irq_sum here.
> >>> Typically, # of possible cpu are very big.
> >>>
> >>> So, I don't like unnecessary twrice looping.
> >>
> >> I was about to send these patches to Linus, but it seems that this
> >> optmisation hasn't been addressed?
> >
> > Thanks for your notice, Andrew.
> > Of course, thanks for your advice, Kosaki-san.
> > I didn't check carefully about this point...
> >
> > Now I am on a business trip.
> > So, I'll post the fix next week.
>
> No problem.
> I've made it three hour ago. I'll post soon.

Done.


==
Subject: [PATCH] proc: remove redundunt for_each_possible_cpu() loop
Impact: cleanup

Almost distro set NR_CPUS very large number and at that time for_each_possible_cpu() is
a bit costly.

Therefore, To remove unnecessary twice loop is better.


Note. we can remove softirq's redundunt loop, but we can't remove irq's.
because NR_SOFTIRQS ~= 10, NR_IRQS ~= 4000 (in x86 case).


Signed-off-by: KOSAKI Motohiro <[email protected]>
Cc: Keika Kobayashi <[email protected]>
Cc: Hiroshi Shimamoto <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Alexey Dobriyan <[email protected]>
Cc: Andrew Morton <[email protected]>
---
fs/proc/stat.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/proc/stat.c b/fs/proc/stat.c
index f95d73a..cb839e8 100644
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -27,6 +27,7 @@ static int show_stat(struct seq_file *p, void *v)
cputime64_t guest;
u64 sum = 0;
u64 sum_softirq = 0;
+ unsigned int per_softirq_sums[NR_SOFTIRQS] = {0};
struct timespec boottime;
unsigned int per_irq_sum;

@@ -51,8 +52,12 @@ static int show_stat(struct seq_file *p, void *v)
}
sum += arch_irq_stat_cpu(i);

- for (j = 0; j < NR_SOFTIRQS; j++)
- sum_softirq += kstat_softirqs_cpu(j, i);
+ for (j = 0; j < NR_SOFTIRQS; j++) {
+ unsigned int softirq_stat = kstat_softirqs_cpu(j, i);
+
+ per_softirq_sums[j] += softirq_stat;
+ sum_softirq += softirq_stat;
+ }

}
sum += arch_irq_stat();
@@ -118,12 +123,7 @@ static int show_stat(struct seq_file *p, void *v)
seq_printf(p, "softirq %llu", (unsigned long long)sum_softirq);

for (i = 0; i < NR_SOFTIRQS; i++) {
- per_irq_sum = 0;
-
- for_each_possible_cpu(j)
- per_irq_sum += kstat_softirqs_cpu(i, j);
-
- seq_printf(p, " %u", per_irq_sum);
+ seq_printf(p, " %u", per_softirq_sums[i]);
}
seq_printf(p, "\n");



2009-01-12 23:53:50

by Keika Kobayashi

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] proc: Export statistics for softirq to /proc

2009/1/12 KOSAKI Motohiro <[email protected]>:
>> >>> You can calcurate per_irq_sum here.
>> >>> Typically, # of possible cpu are very big.
>> >>>
>> >>> So, I don't like unnecessary twrice looping.
>> >>
>> >> I was about to send these patches to Linus, but it seems that this
>> >> optmisation hasn't been addressed?
>> >
>> > Thanks for your notice, Andrew.
>> > Of course, thanks for your advice, Kosaki-san.
>> > I didn't check carefully about this point...
>> >
>> > Now I am on a business trip.
>> > So, I'll post the fix next week.
>>
>> No problem.
>> I've made it three hour ago. I'll post soon.
>
> Done.
>
>
> ==
> Subject: [PATCH] proc: remove redundunt for_each_possible_cpu() loop
> Impact: cleanup
>
> Almost distro set NR_CPUS very large number and at that time for_each_possible_cpu() is
> a bit costly.
>
> Therefore, To remove unnecessary twice loop is better.
>
>
> Note. we can remove softirq's redundunt loop, but we can't remove irq's.
> because NR_SOFTIRQS ~= 10, NR_IRQS ~= 4000 (in x86 case).
>
>
> Signed-off-by: KOSAKI Motohiro <[email protected]>
> Cc: Keika Kobayashi <[email protected]>
> Cc: Hiroshi Shimamoto <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Alexey Dobriyan <[email protected]>
> Cc: Andrew Morton <[email protected]>
> ---
> fs/proc/stat.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/proc/stat.c b/fs/proc/stat.c
> index f95d73a..cb839e8 100644
> --- a/fs/proc/stat.c
> +++ b/fs/proc/stat.c
> @@ -27,6 +27,7 @@ static int show_stat(struct seq_file *p, void *v)
> cputime64_t guest;
> u64 sum = 0;
> u64 sum_softirq = 0;
> + unsigned int per_softirq_sums[NR_SOFTIRQS] = {0};
> struct timespec boottime;
> unsigned int per_irq_sum;
>
> @@ -51,8 +52,12 @@ static int show_stat(struct seq_file *p, void *v)
> }
> sum += arch_irq_stat_cpu(i);
>
> - for (j = 0; j < NR_SOFTIRQS; j++)
> - sum_softirq += kstat_softirqs_cpu(j, i);
> + for (j = 0; j < NR_SOFTIRQS; j++) {
> + unsigned int softirq_stat = kstat_softirqs_cpu(j, i);
> +
> + per_softirq_sums[j] += softirq_stat;
> + sum_softirq += softirq_stat;
> + }
>
> }
> sum += arch_irq_stat();
> @@ -118,12 +123,7 @@ static int show_stat(struct seq_file *p, void *v)
> seq_printf(p, "softirq %llu", (unsigned long long)sum_softirq);
>
> for (i = 0; i < NR_SOFTIRQS; i++) {
> - per_irq_sum = 0;
> -
> - for_each_possible_cpu(j)
> - per_irq_sum += kstat_softirqs_cpu(i, j);
> -
> - seq_printf(p, " %u", per_irq_sum);
> + seq_printf(p, " %u", per_softirq_sums[i]);
> }
> seq_printf(p, "\n");
>

Thank you very much, Kosaki-san!
It looks good to me.

// Keika Kobayashi