All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] softirq: Introduce statistics for softirq
@ 2008-11-21  3:55 Keika Kobayashi
  2008-11-21  3:58 ` [PATCH 2/3] proc: Export statistics for softirq to /proc Keika Kobayashi
  2008-11-21  4:01 ` [PATCH 3/3] proc: Update document for /proc/softirqs and /proc/stat Keika Kobayashi
  0 siblings, 2 replies; 9+ messages in thread
From: Keika Kobayashi @ 2008-11-21  3:55 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: h-shimamoto

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 <kobayashi.kk@ncos.nec.co.jp>
Reviewed-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] proc: Export statistics for softirq to /proc
  2008-11-21  3:55 [PATCH 1/3] softirq: Introduce statistics for softirq Keika Kobayashi
@ 2008-11-21  3:58 ` Keika Kobayashi
  2008-11-21  4:16   ` Alexey Dobriyan
  2008-11-21  5:09   ` Andrew Morton
  2008-11-21  4:01 ` [PATCH 3/3] proc: Update document for /proc/softirqs and /proc/stat Keika Kobayashi
  1 sibling, 2 replies; 9+ messages in thread
From: Keika Kobayashi @ 2008-11-21  3:58 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: h-shimamoto

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 <kobayashi.kk@ncos.nec.co.jp>
Reviewed-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 fs/proc/Makefile          |    1 +
 fs/proc/softirqs.c        |   57 +++++++++++++++++++++++++++++++++++++++++++++
 fs/proc/stat.c            |   17 +++++++++++++
 include/linux/interrupt.h |    3 ++
 4 files changed, 78 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..43d3bc0
--- /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_online_cpu(i)
+		seq_printf(p, "CPU%-8d", i);
+	seq_printf(p, "\n");
+
+	for_each_softirq_nr(i) {
+		seq_printf(p, "%-10s", desc_array[i]);
+		for_each_online_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 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..02d5bf8 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_each_softirq_nr(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_each_softirq_nr(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;
 }
 
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index f58a0cf..9a12ba0 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -259,6 +259,9 @@ enum
 	NR_SOFTIRQS
 };
 
+#define for_each_softirq_nr(irq)		\
+	for (irq = 0; irq < NR_SOFTIRQS; irq++)
+
 /* softirq mask and active fields moved to irq_cpustat_t in
  * asm/hardirq.h to get better cache usage.  KAO
  */
-- 
1.5.0.6


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] proc: Update document for /proc/softirqs and /proc/stat
  2008-11-21  3:55 [PATCH 1/3] softirq: Introduce statistics for softirq Keika Kobayashi
  2008-11-21  3:58 ` [PATCH 2/3] proc: Export statistics for softirq to /proc Keika Kobayashi
@ 2008-11-21  4:01 ` Keika Kobayashi
  1 sibling, 0 replies; 9+ messages in thread
From: Keika Kobayashi @ 2008-11-21  4:01 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: h-shimamoto

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

Signed-off-by: Keika Kobayashi <kobayashi.kk@ncos.nec.co.jp>
Reviewed-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] proc: Export statistics for softirq to /proc
  2008-11-21  3:58 ` [PATCH 2/3] proc: Export statistics for softirq to /proc Keika Kobayashi
@ 2008-11-21  4:16   ` Alexey Dobriyan
  2008-11-21  4:21     ` Randy Dunlap
  2008-11-21  5:09   ` Andrew Morton
  1 sibling, 1 reply; 9+ messages in thread
From: Alexey Dobriyan @ 2008-11-21  4:16 UTC (permalink / raw)
  To: Keika Kobayashi; +Cc: linux-kernel, akpm, h-shimamoto

On Thu, Nov 20, 2008 at 07:58:43PM -0800, Keika Kobayashi wrote:
> Export statistics for softirq in /proc/softirqs and /proc/stat.

I thought we added debugfs to stop adding these and similar stuff to /proc.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] proc: Export statistics for softirq to /proc
  2008-11-21  4:16   ` Alexey Dobriyan
@ 2008-11-21  4:21     ` Randy Dunlap
  0 siblings, 0 replies; 9+ messages in thread
From: Randy Dunlap @ 2008-11-21  4:21 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Keika Kobayashi, linux-kernel, akpm, h-shimamoto

Alexey Dobriyan wrote:
> On Thu, Nov 20, 2008 at 07:58:43PM -0800, Keika Kobayashi wrote:
>> Export statistics for softirq in /proc/softirqs and /proc/stat.
> 
> I thought we added debugfs to stop adding these and similar stuff to /proc.

This doesn't look like debugfs material to me.
It's just another component of /proc/stat or /proc/interrupts or wherever.

-- 
~Randy

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] proc: Export statistics for softirq to /proc
  2008-11-21  3:58 ` [PATCH 2/3] proc: Export statistics for softirq to /proc Keika Kobayashi
  2008-11-21  4:16   ` Alexey Dobriyan
@ 2008-11-21  5:09   ` Andrew Morton
  2008-11-21 18:06     ` Keika Kobayashi
  2008-11-22  1:07     ` Keika Kobayashi
  1 sibling, 2 replies; 9+ messages in thread
From: Andrew Morton @ 2008-11-21  5:09 UTC (permalink / raw)
  To: Keika Kobayashi; +Cc: linux-kernel, h-shimamoto, Alexey Dobriyan

On Thu, 20 Nov 2008 19:58:43 -0800 Keika Kobayashi <kobayashi.kk@ncos.nec.co.jp> wrote:

> 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 <kobayashi.kk@ncos.nec.co.jp>
> Reviewed-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

This all looks reasonable to me.

>  fs/proc/Makefile          |    1 +
>  fs/proc/softirqs.c        |   57 +++++++++++++++++++++++++++++++++++++++++++++
>  fs/proc/stat.c            |   17 +++++++++++++
>  include/linux/interrupt.h |    3 ++
>  4 files changed, 78 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..43d3bc0
> --- /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_online_cpu(i)
> +		seq_printf(p, "CPU%-8d", i);
> +	seq_printf(p, "\n");
> +
> +	for_each_softirq_nr(i) {
> +		seq_printf(p, "%-10s", desc_array[i]);
> +		for_each_online_cpu(j)
> +			seq_printf(p, "%10u ", kstat_softirqs_cpu(i, j));
> +		seq_printf(p, "\n");
> +	}
> +	return 0;
> +}

This uses for_each_online_cpu(), but below we use for_each_possible_cpu().

Shouldn't we be consistent here so that at least the numbers will add
up to the same thing?

Probably for_each_possible_cpu() is best - people might want to see how
many softirqs happened on a CPU which was recently offlined.

> +
> +static int softirqs_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, show_softirqs, NULL);
> +}
> +
> +static struct file_operations proc_softirqs_operations = {

Make this const, please.

> +	.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..02d5bf8 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_each_softirq_nr(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_each_softirq_nr(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;
>  }
>  
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index f58a0cf..9a12ba0 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -259,6 +259,9 @@ enum
>  	NR_SOFTIRQS
>  };
>  
> +#define for_each_softirq_nr(irq)		\
> +	for (irq = 0; irq < NR_SOFTIRQS; irq++)

Can we remove this please?  It doesn't make the code any more readable.
Just open-code the loop at each site.

(And strictly speaking the `irq' macro arg should be parenthesised)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] proc: Export statistics for softirq to /proc
  2008-11-21  5:09   ` Andrew Morton
@ 2008-11-21 18:06     ` Keika Kobayashi
  2008-11-22  1:07     ` Keika Kobayashi
  1 sibling, 0 replies; 9+ messages in thread
From: Keika Kobayashi @ 2008-11-21 18:06 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, h-shimamoto, Alexey Dobriyan

Thank you for your comments.

> > +
> > +/*
> > + * /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_online_cpu(i)
> > +		seq_printf(p, "CPU%-8d", i);
> > +	seq_printf(p, "\n");
> > +
> > +	for_each_softirq_nr(i) {
> > +		seq_printf(p, "%-10s", desc_array[i]);
> > +		for_each_online_cpu(j)
> > +			seq_printf(p, "%10u ", kstat_softirqs_cpu(i, j));
> > +		seq_printf(p, "\n");
> > +	}
> > +	return 0;
> > +}
> 
> This uses for_each_online_cpu(), but below we use for_each_possible_cpu().
> 
> Shouldn't we be consistent here so that at least the numbers will add
> up to the same thing?
> 
> Probably for_each_possible_cpu() is best - people might want to see how
> many softirqs happened on a CPU which was recently offlined.

OK. I'll look into this point.

> > +
> > +static int softirqs_open(struct inode *inode, struct file *file)
> > +{
> > +	return single_open(file, show_softirqs, NULL);
> > +}
> > +
> > +static struct file_operations proc_softirqs_operations = {
> 
> Make this const, please.
> 
> > +	.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..02d5bf8 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_each_softirq_nr(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_each_softirq_nr(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;
> >  }
> >  
> > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> > index f58a0cf..9a12ba0 100644
> > --- a/include/linux/interrupt.h
> > +++ b/include/linux/interrupt.h
> > @@ -259,6 +259,9 @@ enum
> >  	NR_SOFTIRQS
> >  };
> >  
> > +#define for_each_softirq_nr(irq)		\
> > +	for (irq = 0; irq < NR_SOFTIRQS; irq++)
> 
> Can we remove this please?  It doesn't make the code any more readable.
> Just open-code the loop at each site.
> 
> (And strictly speaking the `irq' macro arg should be parenthesised)

I agree with you.
I'll remove this definition.

Later, I'll post v2.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] proc: Export statistics for softirq to /proc
  2008-11-21  5:09   ` Andrew Morton
  2008-11-21 18:06     ` Keika Kobayashi
@ 2008-11-22  1:07     ` Keika Kobayashi
  2008-11-22  1:35       ` Andrew Morton
  1 sibling, 1 reply; 9+ messages in thread
From: Keika Kobayashi @ 2008-11-22  1:07 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, h-shimamoto, Alexey Dobriyan

> > +/*
> > + * /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_online_cpu(i)
> > +		seq_printf(p, "CPU%-8d", i);
> > +	seq_printf(p, "\n");
> > +
> > +	for_each_softirq_nr(i) {
> > +		seq_printf(p, "%-10s", desc_array[i]);
> > +		for_each_online_cpu(j)
> > +			seq_printf(p, "%10u ", kstat_softirqs_cpu(i, j));
> > +		seq_printf(p, "\n");
> > +	}
> > +	return 0;
> > +}
> 
> This uses for_each_online_cpu(), but below we use for_each_possible_cpu().
> 
> Shouldn't we be consistent here so that at least the numbers will add
> up to the same thing?
> 
> Probably for_each_possible_cpu() is best - people might want to see how
> many softirqs happened on a CPU which was recently offlined.

I understand this point. I'll fix it later.

There is same problem regarding /proc/interrupts.
Should we change from for_each_online_cpu() to for_each_possible_cpu(),
or is it too late?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] proc: Export statistics for softirq to /proc
  2008-11-22  1:07     ` Keika Kobayashi
@ 2008-11-22  1:35       ` Andrew Morton
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2008-11-22  1:35 UTC (permalink / raw)
  To: Keika Kobayashi; +Cc: linux-kernel, h-shimamoto, Alexey Dobriyan

On Fri, 21 Nov 2008 17:07:43 -0800 Keika Kobayashi <kobayashi.kk@ncos.nec.co.jp> wrote:

> > This uses for_each_online_cpu(), but below we use for_each_possible_cpu().
> > 
> > Shouldn't we be consistent here so that at least the numbers will add
> > up to the same thing?
> > 
> > Probably for_each_possible_cpu() is best - people might want to see how
> > many softirqs happened on a CPU which was recently offlined.
> 
> I understand this point. I'll fix it later.
> 
> There is same problem regarding /proc/interrupts.
> Should we change from for_each_online_cpu() to for_each_possible_cpu(),
> or is it too late?

I assume that /proc/interrupts has been that way for a very long time,
and nobody has noticed&complained.  If we can find anyone who actually
uses cpu hotplug then perhaps they could help us out here.  But such
people seem to be rare.

I dunno.  I guess we can leave /proc/interrupts alone.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2008-11-22  1:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-21  3:55 [PATCH 1/3] softirq: Introduce statistics for softirq Keika Kobayashi
2008-11-21  3:58 ` [PATCH 2/3] proc: Export statistics for softirq to /proc Keika Kobayashi
2008-11-21  4:16   ` Alexey Dobriyan
2008-11-21  4:21     ` Randy Dunlap
2008-11-21  5:09   ` Andrew Morton
2008-11-21 18:06     ` Keika Kobayashi
2008-11-22  1:07     ` Keika Kobayashi
2008-11-22  1:35       ` Andrew Morton
2008-11-21  4:01 ` [PATCH 3/3] proc: Update document for /proc/softirqs and /proc/stat Keika Kobayashi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.