Re: Reading /proc/stat is slooow

Re: Reading /proc/stat is slooow

[Luck, Tony]

On Tue, Dec 06, 2005 at 05:24:16PM +0800, Zou, Nanhai wrote:
>   I think the reason of second loop much slower than the first one is page coloring.
>   For the first loop, at least cache line is hot in the inner loop.
> For the second loop, things will be much worse because percpu data offset is 64k....

Two possible fixes:
1) Compute the per irq sums during the first (cache-friendly) loop.  This has
the downside that we need to allocate NR_IRQS*sizeof(int) to save the sums
until we need them.  This might not be popular for other architectures who
don't have a big problem here as they don't allow such large values for NR_CPUS.

2) The problem loop is already #ifdef'd out for PPC64 and ALPHA.  We could add
IA64 to that exclusive club and just not include the per irq sums.  Since kstat_irqs()
computes the sums in an "int", they will wrap frequently on a large system
(512 cpus * default 250Hz = 128000 ... which wraps a 32-bit unsigned in 9 hours
and 19 minutes) ... so their usefulness is questionable.  Does xosview use
the per-irq values?

>  But I have no idea of why 2.6.13 is much faster here.
Andreas didn't have CPU_HOTPLUG turned on in his 2.6.13 build.  Which means that
the "for_each_cpu" loop inside kstat_cpus() only touched the percpu area for the
cpus on his system (with CPU_HOTPLUG=n cpu_possible map is equal to cpu_present).

proof of concept patch for option #1 (drops time from 52ms to 3ms with NR_CPUS=1024):

--- a/fs/proc/proc_misc.c	2005-12-05 17:09:25.000000000 -0800
+++ b/fs/proc/proc_misc.c	2005-12-06 08:47:43.000000000 -0800
@@ -343,6 +343,7 @@
 	unsigned long jif;
 	cputime64_t user, nice, system, idle, iowait, irq, softirq, steal;
 	u64 sum = 0;
+	int *perirqsums;
 
 	user = nice = system = idle = iowait =
 		irq = softirq = steal = cputime64_zero;
@@ -350,6 +351,7 @@
 	if (wall_to_monotonic.tv_nsec)
 		--jif;
 
+	perirqsums = kcalloc(NR_IRQS, sizeof (*perirqsums), GFP_KERNEL);
 	for_each_cpu(i) {
 		int j;
 
@@ -361,8 +363,10 @@
 		irq = cputime64_add(irq, kstat_cpu(i).cpustat.irq);
 		softirq = cputime64_add(softirq, kstat_cpu(i).cpustat.softirq);
 		steal = cputime64_add(steal, kstat_cpu(i).cpustat.steal);
-		for (j = 0 ; j < NR_IRQS ; j++)
+		for (j = 0 ; j < NR_IRQS ; j++) {
 			sum += kstat_cpu(i).irqs[j];
+			perirqsums[j] += kstat_cpu(i).irqs[j];
+		}
 	}
 
 	seq_printf(p, "cpu  %llu %llu %llu %llu %llu %llu %llu %llu\n",
@@ -400,8 +404,9 @@
 
 #if !defined(CONFIG_PPC64) && !defined(CONFIG_ALPHA)
 	for (i = 0; i < NR_IRQS; i++)
-		seq_printf(p, " %u", kstat_irqs(i));
+		seq_printf(p, " %u", perirqsums[i]);
 #endif
+	kfree(perirqsums);
 
 	seq_printf(p,
 		"\nctxt %llu\n"

Re: Reading /proc/stat is slooow

[Andreas Schwab]

"Luck, Tony" <tony.luck@intel.com> writes:

> 2) The problem loop is already #ifdef'd out for PPC64 and ALPHA.  We could add
> IA64 to that exclusive club and just not include the per irq sums.  Since kstat_irqs()
> computes the sums in an "int", they will wrap frequently on a large system
> (512 cpus * default 250Hz = 128000 ... which wraps a 32-bit unsigned in 9 hours
> and 19 minutes) ... so their usefulness is questionable.  Does xosview use
> the per-irq values?

It doesn't use them, it uses /proc/interrupts instead.  So IMHO this would
be the preferred solution.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[PATCH] Drop per-irq counters from /proc/stat (Was: Reading /proc/stat is slooow)

[Luck, Tony]

On Tue, Dec 06, 2005 at 06:10:40PM +0100, Andreas Schwab wrote:
> "Luck, Tony" <tony.luck@intel.com> writes:
> 
> > 2) The problem loop is already #ifdef'd out for PPC64 and ALPHA.  We could add
> > IA64 to that exclusive club and just not include the per irq sums.  Since kstat_irqs()
> > computes the sums in an "int", they will wrap frequently on a large system
> > (512 cpus * default 250Hz = 128000 ... which wraps a 32-bit unsigned in 9 hours
> > and 19 minutes) ... so their usefulness is questionable.  Does xosview use
> > the per-irq values?
> 
> It doesn't use them, it uses /proc/interrupts instead.  So IMHO this would
> be the preferred solution.

Anyone know of any applications that *DO* depend on the per-irq counters
in /proc/stat?

I could just add IA64 to the ugly #ifdef:

-#if !defined(CONFIG_PPC64) && !defined(CONFIG_ALPHA)
+#if !defined(CONFIG_PPC64) && !defined(CONFIG_ALPHA) && !defined(CONFIG_IA64)

But perhaps it might be simpler to skip straight to this:

[PATCH] Drop per-irq counters from /proc/stat

It is very expensive to produce the per-irq counter values in /proc/stat
(the code does a very cache unfriendly walk across all percpu structures
for each irq).  Some architectures (PPC64, ALPHA) don't provide this
information in this file.  Applications which do want this can derive it
by adding the percpu values in /proc/interrupts.

Signed-off-by: Tony Luck <tony.luck@intel.com>

---

diff --git a/fs/proc/proc_misc.c b/fs/proc/proc_misc.c
index 5b6b0b6..64d96bf 100644
--- a/fs/proc/proc_misc.c
+++ b/fs/proc/proc_misc.c
@@ -396,15 +396,10 @@ static int show_stat(struct seq_file *p,
 			(unsigned long long)cputime64_to_clock_t(softirq),
 			(unsigned long long)cputime64_to_clock_t(steal));
 	}
-	seq_printf(p, "intr %llu", (unsigned long long)sum);
-
-#if !defined(CONFIG_PPC64) && !defined(CONFIG_ALPHA)
-	for (i = 0; i < NR_IRQS; i++)
-		seq_printf(p, " %u", kstat_irqs(i));
-#endif
+	seq_printf(p, "intr %llu\n", (unsigned long long)sum);
 
 	seq_printf(p,
-		"\nctxt %llu\n"
+		"ctxt %llu\n"
 		"btime %lu\n"
 		"processes %lu\n"
 		"procs_running %lu\n"


-Tony