[PATCH] Re: /proc/uptime idle counter remains at 0

[PATCH] Re: /proc/uptime idle counter remains at 0

[Michael Abbott]

Resending this patch, as it doesn't seem to have made it into the kernel 
(and this feature really doesn't need to be left broken).

---------- Forwarded message ----------
Date: Mon, 11 May 2009 10:07:14 +0100 (BST)
From: Michael Abbott <michael@araneidae.co.uk>
To: Jan Engelhardt <jengelh@medozas.de>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>,
    Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Re: /proc/uptime idle counter remains at 0

On Mon, 11 May 2009, Michael Abbott wrote:
> So as a simple solution, I've attached a patch where I just copy the 
> idle field processing from fs/proc/stat.c.  I expect that on a 
> multi-processor machine things may not be quite so simple -- as up time 
> is in elapsed wall-clock time, then so should idle time be, so we 
> probably need to also divide by the number of processors.  Afraid I 
> don't have a multiprocessor test system, and /proc/stat seems ok, so 
> I've not made this refinement.

To hopefully help, I've rebased the patch onto current git 
(fs/proc/uptime.c has undergone some unrelated changes).


Date: Mon, 11 May 2009 07:14:19 +0100
Subject: [PATCH] Fix idle time field in /proc/uptime

Git commit 79741dd changes idle cputime accounting, but unfortunately
the /proc/uptime file hasn't caught up.  Here the idle time calculation
from /proc/stat is copied over.

Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
---
 fs/proc/uptime.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
index 0c10a0b..0f43395 100644
--- a/fs/proc/uptime.c
+++ b/fs/proc/uptime.c
@@ -4,13 +4,19 @@
 #include <linux/sched.h>
 #include <linux/seq_file.h>
 #include <linux/time.h>
+#include <linux/kernel_stat.h>
 #include <asm/cputime.h>
 
 static int uptime_proc_show(struct seq_file *m, void *v)
 {
 	struct timespec uptime;
 	struct timespec idle;
-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
+	int len, i;
+	cputime_t idletime = 0;
+
+	for_each_possible_cpu(i) 
+		idletime = cputime64_add(idletime, kstat_cpu(i).cpustat.idle);
+	idletime = cputime64_to_clock_t(idletime);
 
 	do_posix_clock_monotonic_gettime(&uptime);
 	monotonic_to_bootbased(&uptime);
-- 
1.6.1.3

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Martin Schwidefsky]

On Mon, 18 May 2009 14:23:03 +0100 (BST)
Michael Abbott <michael@araneidae.co.uk> wrote:

> Resending this patch, as it doesn't seem to have made it into the kernel 
> (and this feature really doesn't need to be left broken).

I'll queue it to the cputime branch on git390 and send Linus a please
pull. Perhaps that will get the patch upstream.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Martin Schwidefsky]

On Mon, 18 May 2009 14:23:03 +0100 (BST)
Michael Abbott <michael@araneidae.co.uk> wrote:

> diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
> index 0c10a0b..0f43395 100644
> --- a/fs/proc/uptime.c
> +++ b/fs/proc/uptime.c
> @@ -4,13 +4,19 @@
>  #include <linux/sched.h>
>  #include <linux/seq_file.h>
>  #include <linux/time.h>
> +#include <linux/kernel_stat.h>
>  #include <asm/cputime.h>
> 
>  static int uptime_proc_show(struct seq_file *m, void *v)
>  {
>  	struct timespec uptime;
>  	struct timespec idle;
> -	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
> +	int len, i;
> +	cputime_t idletime = 0;
> +
> +	for_each_possible_cpu(i) 
> +		idletime = cputime64_add(idletime, kstat_cpu(i).cpustat.idle);
> +	idletime = cputime64_to_clock_t(idletime);
> 
>  	do_posix_clock_monotonic_gettime(&uptime);
>  	monotonic_to_bootbased(&uptime);

I found another problem with this patch: why do you convert the
idletime from cputime_t to clock_t ? The call to cputime_to_timespec
takes a cputime_t and the conversion from cputime to clock_t returns a
value that is way to small on s390. After removing that line it works
for me but I wonder why you added it.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Michael Abbott]

Looks like I dropped the ball on this.  Didn't realise the patch would 
just evaporate...

On Mon, 25 May 2009, Martin Schwidefsky wrote:
> On Mon, 18 May 2009 14:23:03 +0100 (BST)
> Michael Abbott <michael@araneidae.co.uk> wrote:
> 
> > diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
> > index 0c10a0b..0f43395 100644
> > --- a/fs/proc/uptime.c
> > +++ b/fs/proc/uptime.c
> > @@ -4,13 +4,19 @@
> >  #include <linux/sched.h>
> >  #include <linux/seq_file.h>
> >  #include <linux/time.h>
> > +#include <linux/kernel_stat.h>
> >  #include <asm/cputime.h>
> > 
> >  static int uptime_proc_show(struct seq_file *m, void *v)
> >  {
> >  	struct timespec uptime;
> >  	struct timespec idle;
> > -	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
> > +	int len, i;
> > +	cputime_t idletime = 0;
> > +
> > +	for_each_possible_cpu(i) 
> > +		idletime = cputime64_add(idletime, kstat_cpu(i).cpustat.idle);
> > +	idletime = cputime64_to_clock_t(idletime);
> > 
> >  	do_posix_clock_monotonic_gettime(&uptime);
> >  	monotonic_to_bootbased(&uptime);
> 
> I found another problem with this patch: why do you convert the
> idletime from cputime_t to clock_t ? The call to cputime_to_timespec
> takes a cputime_t and the conversion from cputime to clock_t returns a
> value that is way to small on s390. After removing that line it works
> for me but I wonder why you added it.

I've tidied up the patch a trifle, the idle time processing is now very 
nearly an exactly copy of that in fs/proc/stat.c.  

Unfortunately this now begs more questions:

1. Why is idle time processing so different from up time, in particular 
why am I still allowing uptime to overflow in a year and four months or 
so?  

Answer: I'm trying to just fix idle time, and the uptime calculation looks 
like a can of worms to me.

2. Is this really the right calculation of idle time, in particular what 
about multiple CPUs?

Answer: I don't have a multi-process test system, so I'm not qualified to 
investigate.  My view is that idle time should match uptime, ie should be 
in literal elapsed machine time (so should really be divided by the number 
of CPUs here), but it's clear that doing this properly is not 
straighforward.

As for whether it is "safe" to compute idle time this way, this code is 
just a copy of what is done in fs/proc/stat.c, so worry about that first!


It's rather a shame that /proc/uptime idle time reporting has been busted 
now for what will be three kernel releases in a row.


>From b885468d04475522486e7004de5dba29df1a28a8 Mon Sep 17 00:00:00 2001
From: Michael Abbott <michael.abbott@diamond.ac.uk>
Date: Mon, 11 May 2009 07:14:19 +0100
Subject: [PATCH] Fix idle time field in /proc/uptime

Git commit 79741dd changes idle cputime accounting, but unfortunately
the /proc/uptime file hasn't caught up.  Here the idle time calculation
from /proc/stat is copied over.  Further changes from commit e1c8053
are also included in this fix.

Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
---
 fs/proc/uptime.c |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
index 0c10a0b..2d573f7 100644
--- a/fs/proc/uptime.c
+++ b/fs/proc/uptime.c
@@ -4,22 +4,34 @@
 #include <linux/sched.h>
 #include <linux/seq_file.h>
 #include <linux/time.h>
+#include <linux/kernel_stat.h>
 #include <asm/cputime.h>
+#include <asm/div64.h>
+
+#ifndef arch_idle_time
+#define arch_idle_time(cpu) 0
+#endif
 
 static int uptime_proc_show(struct seq_file *m, void *v)
 {
 	struct timespec uptime;
-	struct timespec idle;
-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
+	int i;
+	cputime64_t idle = cputime64_zero;
+	unsigned long int idle_mod;
+
+	for_each_possible_cpu(i) {
+		idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
+		idle = cputime64_add(idle, arch_idle_time(i));
+	}
+	idle = cputime64_to_clock_t(idle);
+	idle_mod = do_div(idle, 100);
 
 	do_posix_clock_monotonic_gettime(&uptime);
 	monotonic_to_bootbased(&uptime);
-	cputime_to_timespec(idletime, &idle);
-	seq_printf(m, "%lu.%02lu %lu.%02lu\n",
+	seq_printf(m, "%lu.%02lu %llu.%02lu\n",
 			(unsigned long) uptime.tv_sec,
 			(uptime.tv_nsec / (NSEC_PER_SEC / 100)),
-			(unsigned long) idle.tv_sec,
-			(idle.tv_nsec / (NSEC_PER_SEC / 100)));
+			idle, idle_mod);
 	return 0;
 }
 
-- 
1.6.1.3

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Jan Engelhardt]

On Monday 2009-07-06 17:48, Michael Abbott wrote:
>[...]
>why am I still allowing uptime to overflow in a year and four months or 
>so?

Is not this addressed by 64-bit jiffies already?

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Michael Abbott]

On Mon, 6 Jul 2009, Jan Engelhardt wrote:
> On Monday 2009-07-06 17:48, Michael Abbott wrote:
> >[...]
> >why am I still allowing uptime to overflow in a year and four months or 
> >so?
> Is not this addressed by 64-bit jiffies already?

Possibly, sorry, not terribly familiar with the underlying infrastructure,
just trying to fix one bug without being dragged under by all the rest!

/proc/uptime idle counter remains at 0

[Jan Engelhardt]

Hi,


starting from v2.6.28-4930-g79741dd lasting thru at least v2.6.29.1,
the second field of /proc/uptime always shows 0.00. This happens for
both the typical i386 (my case) and on an ARM (according to Michael,
cc'ed).

>From the commit log of 79741dd:

	"""The cpu time spent by the idle process actually doing
	something is currently accounted as idle time. This is plain
	wrong, the architectures that support VIRT_CPU_ACCOUNTING=y
	can do better: distinguish between the time spent doing
	nothing and the time spent by idle doing work. The first is
	accounted with account_idle_time and the second with
	account_system_time."""

Citing Michael from our irc conversation:

	"""the writer[committer] [says] that [the] idle process time
	isn't really idle time ... but that's all that /proc/uptime
	looks at. I guess fs/proc/uptime.c needs to catch up."""

So, were the updates to uptime.c missed, or do we now live on with
/proc/uptime constantly having 0?



Jan

Re: /proc/uptime idle counter remains at 0

[Martin Schwidefsky]

On Sat, 9 May 2009 10:05:57 +0200 (CEST)
Jan Engelhardt <jengelh@medozas.de> wrote:

> starting from v2.6.28-4930-g79741dd lasting thru at least v2.6.29.1,
> the second field of /proc/uptime always shows 0.00. This happens for
> both the typical i386 (my case) and on an ARM (according to Michael,
> cc'ed).
> 
> From the commit log of 79741dd:
> 
> 	"""The cpu time spent by the idle process actually doing
> 	something is currently accounted as idle time. This is plain
> 	wrong, the architectures that support VIRT_CPU_ACCOUNTING=y
> 	can do better: distinguish between the time spent doing
> 	nothing and the time spent by idle doing work. The first is
> 	accounted with account_idle_time and the second with
> 	account_system_time."""
> 
> Citing Michael from our irc conversation:
> 
> 	"""the writer[committer] [says] that [the] idle process time
> 	isn't really idle time ... but that's all that /proc/uptime
> 	looks at. I guess fs/proc/uptime.c needs to catch up."""
> 
> So, were the updates to uptime.c missed, or do we now live on with
> /proc/uptime constantly having 0?

The second paragraph from git commit 79741dd tells you more about this:

In addition idle time is no more added to the stime of the idle
process. This field now contains the system time of the idle process as
it should be. On systems without VIRT_CPU_ACCOUNTING this will always
be zero as every tick that occurs while idle is running will be
accounted as idle time.

The point is the semantics of the stime field for the idle process. The
stime field used to contain the real system time (cpu really did
something) of the idle process plus the idle time (cpu is stopped).
After the change the field only contains the real system time. Which is
ihmo much more useful, no?

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: /proc/uptime idle counter remains at 0

[Jan Engelhardt]

On Sunday 2009-05-10 19:12, Martin Schwidefsky wrote:
>> 
>> So, were the updates to uptime.c missed, or do we now live on with
>> /proc/uptime constantly having 0?
>
>The second paragraph from git commit 79741dd tells you more about this:
>
>In addition idle time is no more added to the stime of the idle
>process. This field now contains the system time of the idle process as
>it should be. On systems without VIRT_CPU_ACCOUNTING this will always
>be zero as every tick that occurs while idle is running will be
>accounted as idle time.
>
>The point is the semantics of the stime field for the idle process. The
>stime field used to contain the real system time (cpu really did
>something) of the idle process plus the idle time (cpu is stopped).
>After the change the field only contains the real system time. Which is
>ihmo much more useful, no?

Actually doing something while idle would then probably be limited to
CPUs that have no HLT instruction/state, like ancient i386, right?

Are the semantics of /proc/uptime (more-or-less standardsly) defined
somewhere, e.g. written down into a manual page?

Nevertheless, one could argue that, hypothetically, some people or
their scripts interpreted the second field as the time that there was
no process running; sort of a minimalistic way to tell the average
system use in % beyond the 1/5/15-loadavg counters. So the field could be
kept, or now that 2nd place displays 0.00, be re-added. Depending on
how “standardized” /proc/uptime's format is, the 0.00 could either
stay as second position or move to third position.

> cat /proc/uptime
496468.50 432205.41
> bc -l <<<'100-(432205.41*100/496468.50)' 
12.94 (%)

[PATCH] Re: /proc/uptime idle counter remains at 0

[Michael Abbott]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4287 bytes --]

On Mon, 11 May 2009, Jan Engelhardt wrote:
> On Sunday 2009-05-10 19:12, Martin Schwidefsky wrote:
> >> So, were the updates to uptime.c missed, or do we now live on with 
> >> /proc/uptime constantly having 0?

Please, let's not do this -- it breaks my instrument (which currently 
thinks the processor is overloaded).

> >The second paragraph from git commit 79741dd tells you more about this:
> >
> >In addition idle time is no more added to the stime of the idle 
> >process. This field now contains the system time of the idle process as 
> >it should be. On systems without VIRT_CPU_ACCOUNTING this will always 
> >be zero as every tick that occurs while idle is running will be 
> >accounted as idle time.
> >
> >The point is the semantics of the stime field for the idle process. The 
> >stime field used to contain the real system time (cpu really did 
> >something) of the idle process plus the idle time (cpu is stopped). 
> >After the change the field only contains the real system time. Which is 
> >ihmo much more useful, no?
> 
> Actually doing something while idle would then probably be limited to 
> CPUs that have no HLT instruction/state, like ancient i386, right?
> 
> Are the semantics of /proc/uptime (more-or-less standardsly) defined 
> somewhere, e.g. written down into a manual page?
> 
> Nevertheless, one could argue that, hypothetically, some people or their 
> scripts interpreted the second field as the time that there was no 
> process running; sort of a minimalistic way to tell the average system 
> use in % beyond the 1/5/15-loadavg counters. So the field could be kept, 
> or now that 2nd place displays 0.00, be re-added. Depending on how 
> “standardized” /proc/uptime's format is, the 0.00 could either stay 
> as second position or move to third position.

I have to confess I don't really understand the logic of what's going on 
here -- in particular, what does the idle process do other than account 
for time when the processor has nothing useful to do?  It does seem to me 
now that the .utime and .stime fields are now less than useful -- maybe 
they can be deleted now?

I've always assumed that the second field of /proc/uptime was a simple 
measure of time not spent doing real work, in other words a crude measure 
of spare CPU resources.  My instrument basically uses the the two fields 
of this file to compute a measure of CPU loading so it can raise an alert 
if the CPU doesn't have enough spare (idle) capacity.

So as a simple solution, I've attached a patch where I just copy the idle 
field processing from fs/proc/stat.c.  I expect that on a multi-processor 
machine things may not be quite so simple -- as up time is in elapsed 
wall-clock time, then so should idle time be, so we probably need to also 
divide by the number of processors.  Afraid I don't have a multiprocessor 
test system, and /proc/stat seems ok, so I've not made this refinement.


Date: Mon, 11 May 2009 07:14:19 +0100
Subject: [PATCH] Fix idle time field in /proc/uptime

Git commit 79741dd changes idle cputime accounting, but unfortunately
the /proc/uptime file hasn't caught up.  Here the idle time calculation
from /proc/stat is copied over.

Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
---
 fs/proc/uptime.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
index df26aa8..0d531bf 100644
--- a/fs/proc/uptime.c
+++ b/fs/proc/uptime.c
@@ -2,6 +2,7 @@
 #include <linux/proc_fs.h>
 #include <linux/sched.h>
 #include <linux/time.h>
+#include <linux/kernel_stat.h>
 #include <asm/cputime.h>
 
 static int proc_calc_metrics(char *page, char **start, off_t off,
@@ -23,8 +24,12 @@ static int uptime_read_proc(char *page, char **start, off_t off, int count,
 {
 	struct timespec uptime;
 	struct timespec idle;
-	int len;
-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
+	int len, i;
+	cputime_t idletime = 0;
+
+	for_each_possible_cpu(i) 
+		idletime = cputime64_add(idletime, kstat_cpu(i).cpustat.idle);
+	idletime = cputime64_to_clock_t(idletime);
 
 	do_posix_clock_monotonic_gettime(&uptime);
 	monotonic_to_bootbased(&uptime);
-- 
1.6.1.3

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Martin Schwidefsky]

On Mon, 11 May 2009 07:23:58 +0100 (BST)
Michael Abbott <michael@araneidae.co.uk> wrote:

> On Mon, 11 May 2009, Jan Engelhardt wrote:
> > On Sunday 2009-05-10 19:12, Martin Schwidefsky wrote:
> > >> So, were the updates to uptime.c missed, or do we now live on with 
> > >> /proc/uptime constantly having 0?
> 
> Please, let's not do this -- it breaks my instrument (which currently 
> thinks the processor is overloaded).

Hmm, bad..

> I have to confess I don't really understand the logic of what's going on 
> here -- in particular, what does the idle process do other than account 
> for time when the processor has nothing useful to do?  It does seem to me 
> now that the .utime and .stime fields are now less than useful -- maybe 
> they can be deleted now?

The idle task does not just sleep, it will do "real" work, e.g. softirq
handling for interrupts / network traffic. This is added to the stime
field of the idle process and it does carry some information. And
deleting these fields won't be possible as utime/stime are needed for
the other processes as well. To selectively remove them for the idle
process doesn't make sense to me.
 
> I've always assumed that the second field of /proc/uptime was a simple 
> measure of time not spent doing real work, in other words a crude measure 
> of spare CPU resources.  My instrument basically uses the the two fields 
> of this file to compute a measure of CPU loading so it can raise an alert 
> if the CPU doesn't have enough spare (idle) capacity.

But it wasn't.. the old style stime of idle is a mixture of true idle
time and some system time.

> So as a simple solution, I've attached a patch where I just copy the idle 
> field processing from fs/proc/stat.c.  I expect that on a multi-processor 
> machine things may not be quite so simple -- as up time is in elapsed 
> wall-clock time, then so should idle time be, so we probably need to also 
> divide by the number of processors.  Afraid I don't have a multiprocessor 
> test system, and /proc/stat seems ok, so I've not made this refinement.

Yes, on a multiprocessor in particular with cpu hotplug you need
additional information to be able to interpret the number. I still like
the patch because it gives the field a defined semantic: the total time
spent by the activated cpus waiting for work measured in cputime terms.
The semantic is still different from the old style number though.

For whom it matters:
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Jan Engelhardt]

On Monday 2009-05-11 09:35, Martin Schwidefsky wrote:
>> I have to confess I don't really understand the logic of what's going on 
>> here -- in particular, what does the idle process do other than account 
>> for time when the processor has nothing useful to do?  It does seem to me 
>> now that the .utime and .stime fields are now less than useful -- maybe 
>> they can be deleted now?
>
>The idle task does not just sleep, it will do "real" work, e.g. softirq
>handling for interrupts / network traffic.

With an RT kernel, SIRQ and such are separate tasks. That should also
be play a role while 0 is displayed.

>> I've always assumed that the second field of /proc/uptime was a simple 
>> measure of time not spent doing real work, in other words a crude measure 
>> of spare CPU resources.  My instrument basically uses the the two fields 
>> of this file to compute a measure of CPU loading so it can raise an alert 
>> if the CPU doesn't have enough spare (idle) capacity.
>
>But it wasn't.. the old style stime of idle is a mixture of true idle
>time and some system time.

Then I would say let's show the idle time only (but still inculde
the idle time when the CPU is halted).

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Martin Schwidefsky]

On Mon, 11 May 2009 09:42:03 +0200 (CEST)
Jan Engelhardt <jengelh@medozas.de> wrote:

> 
> On Monday 2009-05-11 09:35, Martin Schwidefsky wrote:
> >> I have to confess I don't really understand the logic of what's going on 
> >> here -- in particular, what does the idle process do other than account 
> >> for time when the processor has nothing useful to do?  It does seem to me 
> >> now that the .utime and .stime fields are now less than useful -- maybe 
> >> they can be deleted now?
> >
> >The idle task does not just sleep, it will do "real" work, e.g. softirq
> >handling for interrupts / network traffic.
> 
> With an RT kernel, SIRQ and such are separate tasks. That should also
> be play a role while 0 is displayed.

With an RT kernel there still is a tiny amount of system work in idle,
that is the instruction executed to switch to the idle process, go to
sleep, wake up again and switch back to another process.

> >> I've always assumed that the second field of /proc/uptime was a simple 
> >> measure of time not spent doing real work, in other words a crude measure 
> >> of spare CPU resources.  My instrument basically uses the the two fields 
> >> of this file to compute a measure of CPU loading so it can raise an alert 
> >> if the CPU doesn't have enough spare (idle) capacity.
> >
> >But it wasn't.. the old style stime of idle is a mixture of true idle
> >time and some system time.
> 
> Then I would say let's show the idle time only (but still inculde
> the idle time when the CPU is halted).

That is hard to do. I've implemented this for s390 where the idle time
really is the time spent waiting. To improve x86 with its tick based
accounting you need to find out if the timer interrupt arrived while
the cpu has been waiting, e.g. check the instruction pointed to by eip
for "hlt".

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Michael Abbott]

On Mon, 11 May 2009, Michael Abbott wrote:
> So as a simple solution, I've attached a patch where I just copy the 
> idle field processing from fs/proc/stat.c.  I expect that on a 
> multi-processor machine things may not be quite so simple -- as up time 
> is in elapsed wall-clock time, then so should idle time be, so we 
> probably need to also divide by the number of processors.  Afraid I 
> don't have a multiprocessor test system, and /proc/stat seems ok, so 
> I've not made this refinement.

To hopefully help, I've rebased the patch onto current git 
(fs/proc/uptime.c has undergone some unrelated changes).


Date: Mon, 11 May 2009 07:14:19 +0100
Subject: [PATCH] Fix idle time field in /proc/uptime

Git commit 79741dd changes idle cputime accounting, but unfortunately
the /proc/uptime file hasn't caught up.  Here the idle time calculation
from /proc/stat is copied over.

Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
---
 fs/proc/uptime.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
index 0c10a0b..0f43395 100644
--- a/fs/proc/uptime.c
+++ b/fs/proc/uptime.c
@@ -4,13 +4,19 @@
 #include <linux/sched.h>
 #include <linux/seq_file.h>
 #include <linux/time.h>
+#include <linux/kernel_stat.h>
 #include <asm/cputime.h>
 
 static int uptime_proc_show(struct seq_file *m, void *v)
 {
 	struct timespec uptime;
 	struct timespec idle;
-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
+	int len, i;
+	cputime_t idletime = 0;
+
+	for_each_possible_cpu(i) 
+		idletime = cputime64_add(idletime, kstat_cpu(i).cpustat.idle);
+	idletime = cputime64_to_clock_t(idletime);
 
 	do_posix_clock_monotonic_gettime(&uptime);
 	monotonic_to_bootbased(&uptime);
-- 
1.6.1.3

[PATCH] Re: /proc/uptime idle counter remains at 0

[Michael Abbott]

Reviving this:

On Sat, 9 May 2009, Jan Engelhardt wrote:
> starting from v2.6.28-4930-g79741dd lasting thru at least v2.6.29.1,
> the second field of /proc/uptime always shows 0.00. This happens for
> both the typical i386 (my case) and on an ARM (according to Michael,
> cc'ed).
> 
> >From the commit log of 79741dd:
> 
> 	"""The cpu time spent by the idle process actually doing
> 	something is currently accounted as idle time. This is plain
> 	wrong, the architectures that support VIRT_CPU_ACCOUNTING=y
> 	can do better: distinguish between the time spent doing
> 	nothing and the time spent by idle doing work. The first is
> 	accounted with account_idle_time and the second with
> 	account_system_time."""
> 
> Citing Michael from our irc conversation:
> 
> 	"""the writer[committer] [says] that [the] idle process time
> 	isn't really idle time ... but that's all that /proc/uptime
> 	looks at. I guess fs/proc/uptime.c needs to catch up."""
> 
> So, were the updates to uptime.c missed, or do we now live on with
> /proc/uptime constantly having 0?

My previous patch seems to have run into the sand.  It every so nearly got 
pulled into mainstream as far as I can tell, but didn't seem to make it; 
no idea what happened.

So here we go again:

commit 6d67e34f45a92f347388e35bd84bf0361e660d3b
Author: Michael Abbott <michael.abbott@diamond.ac.uk>
Date:   Mon May 11 07:14:19 2009 +0100

    Fix idle time field in /proc/uptime
    
    Git commit 79741dd changes idle cputime accounting, but unfortunately
    the /proc/uptime file hasn't caught up.  Here the idle time calculation
    from /proc/stat is copied over.  Further changes from commit e1c8053
    are also included in this fix.
    
    Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>

diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
index 0c10a0b..be286b4 100644
--- a/fs/proc/uptime.c
+++ b/fs/proc/uptime.c
@@ -4,22 +4,32 @@
 #include <linux/sched.h>
 #include <linux/seq_file.h>
 #include <linux/time.h>
+#include <linux/kernel_stat.h>
 #include <asm/cputime.h>
+#include <asm/div64.h>
 
 static int uptime_proc_show(struct seq_file *m, void *v)
 {
 	struct timespec uptime;
-	struct timespec idle;
-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
+	int i;
+	cputime64_t idle = cputime64_zero;
+	unsigned long int idle_mod;
+
+	for_each_possible_cpu(i) {
+		idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
+#ifdef arch_idle_time
+		idle = cputime64_add(idle, arch_idle_time(i));
+#endif
+	}
+	idle = cputime64_to_clock_t(idle);
+	idle_mod = do_div(idle, 100);
 
 	do_posix_clock_monotonic_gettime(&uptime);
 	monotonic_to_bootbased(&uptime);
-	cputime_to_timespec(idletime, &idle);
-	seq_printf(m, "%lu.%02lu %lu.%02lu\n",
+	seq_printf(m, "%lu.%02lu %llu.%02lu\n",
 			(unsigned long) uptime.tv_sec,
 			(uptime.tv_nsec / (NSEC_PER_SEC / 100)),
-			(unsigned long) idle.tv_sec,
-			(idle.tv_nsec / (NSEC_PER_SEC / 100)));
+			idle, idle_mod);
 	return 0;
 }
 

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Amerigo Wang]

On Fri, Aug 14, 2009 at 01:18:08PM +0100, Michael Abbott wrote:
>Reviving this:
>
>On Sat, 9 May 2009, Jan Engelhardt wrote:
>> starting from v2.6.28-4930-g79741dd lasting thru at least v2.6.29.1,
>> the second field of /proc/uptime always shows 0.00. This happens for
>> both the typical i386 (my case) and on an ARM (according to Michael,
>> cc'ed).
>> 
>> >From the commit log of 79741dd:
>> 
>> 	"""The cpu time spent by the idle process actually doing
>> 	something is currently accounted as idle time. This is plain
>> 	wrong, the architectures that support VIRT_CPU_ACCOUNTING=y
>> 	can do better: distinguish between the time spent doing
>> 	nothing and the time spent by idle doing work. The first is
>> 	accounted with account_idle_time and the second with
>> 	account_system_time."""
>> 
>> Citing Michael from our irc conversation:
>> 
>> 	"""the writer[committer] [says] that [the] idle process time
>> 	isn't really idle time ... but that's all that /proc/uptime
>> 	looks at. I guess fs/proc/uptime.c needs to catch up."""
>> 
>> So, were the updates to uptime.c missed, or do we now live on with
>> /proc/uptime constantly having 0?
>
>My previous patch seems to have run into the sand.  It every so nearly got 
>pulled into mainstream as far as I can tell, but didn't seem to make it; 
>no idea what happened.
>
>So here we go again:
>
>commit 6d67e34f45a92f347388e35bd84bf0361e660d3b
>Author: Michael Abbott <michael.abbott@diamond.ac.uk>
>Date:   Mon May 11 07:14:19 2009 +0100
>
>    Fix idle time field in /proc/uptime
>    
>    Git commit 79741dd changes idle cputime accounting, but unfortunately
>    the /proc/uptime file hasn't caught up.  Here the idle time calculation
>    from /proc/stat is copied over.  Further changes from commit e1c8053
>    are also included in this fix.
>    
>    Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
>
>diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
>index 0c10a0b..be286b4 100644
>--- a/fs/proc/uptime.c
>+++ b/fs/proc/uptime.c
>@@ -4,22 +4,32 @@
> #include <linux/sched.h>
> #include <linux/seq_file.h>
> #include <linux/time.h>
>+#include <linux/kernel_stat.h>
> #include <asm/cputime.h>
>+#include <asm/div64.h>
> 
> static int uptime_proc_show(struct seq_file *m, void *v)
> {
> 	struct timespec uptime;
>-	struct timespec idle;
>-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
>+	int i;
>+	cputime64_t idle = cputime64_zero;
>+	unsigned long int idle_mod;
>+
>+	for_each_possible_cpu(i) {
>+		idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
>+#ifdef arch_idle_time
>+		idle = cputime64_add(idle, arch_idle_time(i));
>+#endif


This ugly #ifdef can be removed, check fs/proc/stat.c.

Thanks.


>+	}
>+	idle = cputime64_to_clock_t(idle);
>+	idle_mod = do_div(idle, 100);
> 
> 	do_posix_clock_monotonic_gettime(&uptime);
> 	monotonic_to_bootbased(&uptime);
>-	cputime_to_timespec(idletime, &idle);
>-	seq_printf(m, "%lu.%02lu %lu.%02lu\n",
>+	seq_printf(m, "%lu.%02lu %llu.%02lu\n",
> 			(unsigned long) uptime.tv_sec,
> 			(uptime.tv_nsec / (NSEC_PER_SEC / 100)),
>-			(unsigned long) idle.tv_sec,
>-			(idle.tv_nsec / (NSEC_PER_SEC / 100)));
>+			idle, idle_mod);
> 	return 0;
> }
> 
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Michael Abbott]

On Mon, 17 Aug 2009, Amerigo Wang wrote:
> On Fri, Aug 14, 2009 at 01:18:08PM +0100, Michael Abbott wrote:
> >commit 6d67e34f45a92f347388e35bd84bf0361e660d3b
> >Author: Michael Abbott <michael.abbott@diamond.ac.uk>
> >Date:   Mon May 11 07:14:19 2009 +0100
> >
> >    Fix idle time field in /proc/uptime
> >    
> >    Git commit 79741dd changes idle cputime accounting, but unfortunately
> >    the /proc/uptime file hasn't caught up.  Here the idle time calculation
> >    from /proc/stat is copied over.  Further changes from commit e1c8053
> >    are also included in this fix.
> >    
> >    Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
> >
> >diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
> >index 0c10a0b..be286b4 100644
> >--- a/fs/proc/uptime.c
> >+++ b/fs/proc/uptime.c
> >@@ -4,22 +4,32 @@
> > #include <linux/sched.h>
> > #include <linux/seq_file.h>
> > #include <linux/time.h>
> >+#include <linux/kernel_stat.h>
> > #include <asm/cputime.h>
> >+#include <asm/div64.h>
> > 
> > static int uptime_proc_show(struct seq_file *m, void *v)
> > {
> > 	struct timespec uptime;
> >-	struct timespec idle;
> >-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
> >+	int i;
> >+	cputime64_t idle = cputime64_zero;
> >+	unsigned long int idle_mod;
> >+
> >+	for_each_possible_cpu(i) {
> >+		idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
> >+#ifdef arch_idle_time
> >+		idle = cputime64_add(idle, arch_idle_time(i));
> >+#endif
> 
> 
> This ugly #ifdef can be removed, check fs/proc/stat.c.

I'm sorry?  Are you sure?  Here is what fs/proc/stat.c has to say:

#ifndef arch_idle_time
#define arch_idle_time(cpu) 0
#endif
...
		idle = cputime64_add(idle, arch_idle_time(i));


I think what you're actually saying is the #ifdef can be moved to 
somewhere where it can be easily missed.  For this very reason, I'd rather 
be more explicit about it.


> >+	}
> >+	idle = cputime64_to_clock_t(idle);
> >+	idle_mod = do_div(idle, 100);
> > 
> > 	do_posix_clock_monotonic_gettime(&uptime);
> > 	monotonic_to_bootbased(&uptime);
> >-	cputime_to_timespec(idletime, &idle);
> >-	seq_printf(m, "%lu.%02lu %lu.%02lu\n",
> >+	seq_printf(m, "%lu.%02lu %llu.%02lu\n",
> > 			(unsigned long) uptime.tv_sec,
> > 			(uptime.tv_nsec / (NSEC_PER_SEC / 100)),
> >-			(unsigned long) idle.tv_sec,
> >-			(idle.tv_nsec / (NSEC_PER_SEC / 100)));
> >+			idle, idle_mod);
> > 	return 0;
> > }
> > 
> >--

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Amerigo Wang]

On Mon, Aug 17, 2009 at 07:12:36AM +0100, Michael Abbott wrote:
>On Mon, 17 Aug 2009, Amerigo Wang wrote:
>> On Fri, Aug 14, 2009 at 01:18:08PM +0100, Michael Abbott wrote:
>> >commit 6d67e34f45a92f347388e35bd84bf0361e660d3b
>> >Author: Michael Abbott <michael.abbott@diamond.ac.uk>
>> >Date:   Mon May 11 07:14:19 2009 +0100
>> >
>> >    Fix idle time field in /proc/uptime
>> >    
>> >    Git commit 79741dd changes idle cputime accounting, but unfortunately
>> >    the /proc/uptime file hasn't caught up.  Here the idle time calculation
>> >    from /proc/stat is copied over.  Further changes from commit e1c8053
>> >    are also included in this fix.
>> >    
>> >    Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
>> >
>> >diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
>> >index 0c10a0b..be286b4 100644
>> >--- a/fs/proc/uptime.c
>> >+++ b/fs/proc/uptime.c
>> >@@ -4,22 +4,32 @@
>> > #include <linux/sched.h>
>> > #include <linux/seq_file.h>
>> > #include <linux/time.h>
>> >+#include <linux/kernel_stat.h>
>> > #include <asm/cputime.h>
>> >+#include <asm/div64.h>
>> > 
>> > static int uptime_proc_show(struct seq_file *m, void *v)
>> > {
>> > 	struct timespec uptime;
>> >-	struct timespec idle;
>> >-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
>> >+	int i;
>> >+	cputime64_t idle = cputime64_zero;
>> >+	unsigned long int idle_mod;
>> >+
>> >+	for_each_possible_cpu(i) {
>> >+		idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
>> >+#ifdef arch_idle_time
>> >+		idle = cputime64_add(idle, arch_idle_time(i));
>> >+#endif
>> 
>> 
>> This ugly #ifdef can be removed, check fs/proc/stat.c.
>
>I'm sorry?  Are you sure?  Here is what fs/proc/stat.c has to say:
>
>#ifndef arch_idle_time
>#define arch_idle_time(cpu) 0
>#endif
>...
>		idle = cputime64_add(idle, arch_idle_time(i));
>
>
>I think what you're actually saying is the #ifdef can be moved to 
>somewhere where it can be easily missed.  For this very reason, I'd rather 
>be more explicit about it.

Yes, indeed.

I was suggesting to move that #ifdef into some header so that
both two files can use it. For me, this is the better fix.

Thanks.

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Michael Abbott]

On Mon, 17 Aug 2009, Amerigo Wang wrote:
> On Mon, Aug 17, 2009 at 07:12:36AM +0100, Michael Abbott wrote:
> >On Mon, 17 Aug 2009, Amerigo Wang wrote:
> >> On Fri, Aug 14, 2009 at 01:18:08PM +0100, Michael Abbott wrote:
> >> >commit 6d67e34f45a92f347388e35bd84bf0361e660d3b
> >> >Author: Michael Abbott <michael.abbott@diamond.ac.uk>
> >> >Date:   Mon May 11 07:14:19 2009 +0100
> >> >
> >> >    Fix idle time field in /proc/uptime
> >> >    
> >> >    Git commit 79741dd changes idle cputime accounting, but unfortunately
> >> >    the /proc/uptime file hasn't caught up.  Here the idle time calculation
> >> >    from /proc/stat is copied over.  Further changes from commit e1c8053
> >> >    are also included in this fix.
> >> >    
> >> >    Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
> >> >
> >> >diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
> >> >index 0c10a0b..be286b4 100644
> >> >--- a/fs/proc/uptime.c
> >> >+++ b/fs/proc/uptime.c
> >> >@@ -4,22 +4,32 @@
> >> > #include <linux/sched.h>
> >> > #include <linux/seq_file.h>
> >> > #include <linux/time.h>
> >> >+#include <linux/kernel_stat.h>
> >> > #include <asm/cputime.h>
> >> >+#include <asm/div64.h>
> >> > 
> >> > static int uptime_proc_show(struct seq_file *m, void *v)
> >> > {
> >> > 	struct timespec uptime;
> >> >-	struct timespec idle;
> >> >-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
> >> >+	int i;
> >> >+	cputime64_t idle = cputime64_zero;
> >> >+	unsigned long int idle_mod;
> >> >+
> >> >+	for_each_possible_cpu(i) {
> >> >+		idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
> >> >+#ifdef arch_idle_time
> >> >+		idle = cputime64_add(idle, arch_idle_time(i));
> >> >+#endif
> >> 
> >> 
> >> This ugly #ifdef can be removed, check fs/proc/stat.c.
> >
> >I'm sorry?  Are you sure?  Here is what fs/proc/stat.c has to say:
> >
> >#ifndef arch_idle_time
> >#define arch_idle_time(cpu) 0
> >#endif
> >...
> >		idle = cputime64_add(idle, arch_idle_time(i));
> >
> >
> >I think what you're actually saying is the #ifdef can be moved to 
> >somewhere where it can be easily missed.  For this very reason, I'd rather 
> >be more explicit about it.
> 
> Yes, indeed.
> 
> I was suggesting to move that #ifdef into some header so that
> both two files can use it. For me, this is the better fix.

Yes, you're right, this is a much better idea.  In fact, the definition of 
the arch_idle_time symbol itself should be fixed.

My problem here is I'm not getting much traction getting little patches 
like this in;  your suggestion is almost to make this patch larger!


Oh, that's nasty.  Looks as if arch_idle_time only exists in 
	arch/s390/include/asm/cputime.h
and there are 20 other architecture specific cputime.h files as well as 
asm-generic/cputime.h.  I guess asm-generic/cputime.h is only included if 
an architecture specific cputime.h doesn't exist, so it would seem that 
the direct solution would be to add
	#define arch_idle_time(cpu) 0
to *all* the other cputime.h files.  Ewww.

I'm sorry, I really don't know how detailed cpu time accounting works, and 
I have no confidence of getting a patch in after the necessary work has 
been done.


The thing that seems to be killing this patch is all the extra associated 
issues.  It looks as if detailed CPU accounting is actually in a bit of a 
mess ... but I am wholly unqualified to work on this (for a start, all my 
work is uniprocessor!)

Could we just get this patch in, please, and then maybe use this as a 
trigger to clean up everything else?

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Amerigo Wang]

On Mon, Aug 17, 2009 at 07:58:29AM +0100, Michael Abbott wrote:
>On Mon, 17 Aug 2009, Amerigo Wang wrote:
>> On Mon, Aug 17, 2009 at 07:12:36AM +0100, Michael Abbott wrote:
>> >On Mon, 17 Aug 2009, Amerigo Wang wrote:
>> >> On Fri, Aug 14, 2009 at 01:18:08PM +0100, Michael Abbott wrote:
>> >> >commit 6d67e34f45a92f347388e35bd84bf0361e660d3b
>> >> >Author: Michael Abbott <michael.abbott@diamond.ac.uk>
>> >> >Date:   Mon May 11 07:14:19 2009 +0100
>> >> >
>> >> >    Fix idle time field in /proc/uptime
>> >> >    
>> >> >    Git commit 79741dd changes idle cputime accounting, but unfortunately
>> >> >    the /proc/uptime file hasn't caught up.  Here the idle time calculation
>> >> >    from /proc/stat is copied over.  Further changes from commit e1c8053
>> >> >    are also included in this fix.
>> >> >    
>> >> >    Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
>> >> >
>> >> >diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
>> >> >index 0c10a0b..be286b4 100644
>> >> >--- a/fs/proc/uptime.c
>> >> >+++ b/fs/proc/uptime.c
>> >> >@@ -4,22 +4,32 @@
>> >> > #include <linux/sched.h>
>> >> > #include <linux/seq_file.h>
>> >> > #include <linux/time.h>
>> >> >+#include <linux/kernel_stat.h>
>> >> > #include <asm/cputime.h>
>> >> >+#include <asm/div64.h>
>> >> > 
>> >> > static int uptime_proc_show(struct seq_file *m, void *v)
>> >> > {
>> >> > 	struct timespec uptime;
>> >> >-	struct timespec idle;
>> >> >-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
>> >> >+	int i;
>> >> >+	cputime64_t idle = cputime64_zero;
>> >> >+	unsigned long int idle_mod;
>> >> >+
>> >> >+	for_each_possible_cpu(i) {
>> >> >+		idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
>> >> >+#ifdef arch_idle_time
>> >> >+		idle = cputime64_add(idle, arch_idle_time(i));
>> >> >+#endif
>> >> 
>> >> 
>> >> This ugly #ifdef can be removed, check fs/proc/stat.c.
>> >
>> >I'm sorry?  Are you sure?  Here is what fs/proc/stat.c has to say:
>> >
>> >#ifndef arch_idle_time
>> >#define arch_idle_time(cpu) 0
>> >#endif
>> >...
>> >		idle = cputime64_add(idle, arch_idle_time(i));
>> >
>> >
>> >I think what you're actually saying is the #ifdef can be moved to 
>> >somewhere where it can be easily missed.  For this very reason, I'd rather 
>> >be more explicit about it.
>> 
>> Yes, indeed.
>> 
>> I was suggesting to move that #ifdef into some header so that
>> both two files can use it. For me, this is the better fix.
>
>Yes, you're right, this is a much better idea.  In fact, the definition of 
>the arch_idle_time symbol itself should be fixed.
>
>My problem here is I'm not getting much traction getting little patches 
>like this in;  your suggestion is almost to make this patch larger!


You can send it in another patch. ;)


>
>
>Oh, that's nasty.  Looks as if arch_idle_time only exists in 
>	arch/s390/include/asm/cputime.h
>and there are 20 other architecture specific cputime.h files as well as 
>asm-generic/cputime.h.  I guess asm-generic/cputime.h is only included if 
>an architecture specific cputime.h doesn't exist, so it would seem that 
>the direct solution would be to add
>	#define arch_idle_time(cpu) 0
>to *all* the other cputime.h files.  Ewww.


Yeah, include/asm-generic/cputime.h is the right place to add it, I think.


>
>Could we just get this patch in, please, and then maybe use this as a 
>trigger to clean up everything else?

Well, you can separate the patch and send them together.

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Andrew Morton]

On Fri, 14 Aug 2009 13:18:08 +0100 (BST) Michael Abbott <michael@araneidae.co.uk> wrote:

> Reviving this:
> 
> On Sat, 9 May 2009, Jan Engelhardt wrote:
> > starting from v2.6.28-4930-g79741dd lasting thru at least v2.6.29.1,
> > the second field of /proc/uptime always shows 0.00. This happens for
> > both the typical i386 (my case) and on an ARM (according to Michael,
> > cc'ed).
> > 
> > >From the commit log of 79741dd:
> > 
> > 	"""The cpu time spent by the idle process actually doing
> > 	something is currently accounted as idle time. This is plain
> > 	wrong, the architectures that support VIRT_CPU_ACCOUNTING=y
> > 	can do better: distinguish between the time spent doing
> > 	nothing and the time spent by idle doing work. The first is
> > 	accounted with account_idle_time and the second with
> > 	account_system_time."""
> > 
> > Citing Michael from our irc conversation:
> > 
> > 	"""the writer[committer] [says] that [the] idle process time
> > 	isn't really idle time ... but that's all that /proc/uptime
> > 	looks at. I guess fs/proc/uptime.c needs to catch up."""
> > 
> > So, were the updates to uptime.c missed, or do we now live on with
> > /proc/uptime constantly having 0?
> 
> My previous patch seems to have run into the sand.  It every so nearly got 
> pulled into mainstream as far as I can tell, but didn't seem to make it; 
> no idea what happened.
> 
> So here we go again:
> 

Imagine my surprise to find a version of this patch lurking in Martin's
tree since June 22.  It's a regression fix!

Johan, does this patch help with the regression you reported in
http://bugzilla.kernel.org/show_bug.cgi?id=14131 ?

Thanks.


commit 27bf8712477db47c891e6198000c985631cd18de
Author:     Michael Abbott <michael.abbott@diamond.ac.uk>
AuthorDate: Mon Jun 22 12:19:25 2009 +0200
Commit:     Martin Schwidefsky <schwidefsky@de.ibm.com>
CommitDate: Mon Jun 22 12:20:44 2009 +0200

    [PATCH] Fix idle time field in /proc/uptime
    
    Git commit 79741dd changes idle cputime accounting, but unfortunately
    the /proc/uptime file hasn't caught up.  Here the idle time calculation
    from /proc/stat is copied over.
    
    Signed-off-by: Michael Abbott <michael.abbott@diamond.ac.uk>
    Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
index 0c10a0b..766b1d4 100644
--- a/fs/proc/uptime.c
+++ b/fs/proc/uptime.c
@@ -4,13 +4,18 @@
 #include <linux/sched.h>
 #include <linux/seq_file.h>
 #include <linux/time.h>
+#include <linux/kernel_stat.h>
 #include <asm/cputime.h>
 
 static int uptime_proc_show(struct seq_file *m, void *v)
 {
 	struct timespec uptime;
 	struct timespec idle;
-	cputime_t idletime = cputime_add(init_task.utime, init_task.stime);
+	int i;
+	cputime_t idletime = cputime_zero;
+
+	for_each_possible_cpu(i)
+		idletime = cputime64_add(idletime, kstat_cpu(i).cpustat.idle);
 
 	do_posix_clock_monotonic_gettime(&uptime);
 	monotonic_to_bootbased(&uptime);

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Martin Schwidefsky]

On Tue, 8 Sep 2009 22:58:58 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 14 Aug 2009 13:18:08 +0100 (BST) Michael Abbott <michael@araneidae.co.uk> wrote:
> 
> > Reviving this:
> > 
> > On Sat, 9 May 2009, Jan Engelhardt wrote:
> > > starting from v2.6.28-4930-g79741dd lasting thru at least v2.6.29.1,
> > > the second field of /proc/uptime always shows 0.00. This happens for
> > > both the typical i386 (my case) and on an ARM (according to Michael,
> > > cc'ed).
> > > 
> > > >From the commit log of 79741dd:
> > > 
> > > 	"""The cpu time spent by the idle process actually doing
> > > 	something is currently accounted as idle time. This is plain
> > > 	wrong, the architectures that support VIRT_CPU_ACCOUNTING=y
> > > 	can do better: distinguish between the time spent doing
> > > 	nothing and the time spent by idle doing work. The first is
> > > 	accounted with account_idle_time and the second with
> > > 	account_system_time."""
> > > 
> > > Citing Michael from our irc conversation:
> > > 
> > > 	"""the writer[committer] [says] that [the] idle process time
> > > 	isn't really idle time ... but that's all that /proc/uptime
> > > 	looks at. I guess fs/proc/uptime.c needs to catch up."""
> > > 
> > > So, were the updates to uptime.c missed, or do we now live on with
> > > /proc/uptime constantly having 0?
> > 
> > My previous patch seems to have run into the sand.  It every so nearly got 
> > pulled into mainstream as far as I can tell, but didn't seem to make it; 
> > no idea what happened.
> > 
> > So here we go again:
> > 
> 
> Imagine my surprise to find a version of this patch lurking in Martin's
> tree since June 22.  It's a regression fix!
> 
> Johan, does this patch help with the regression you reported in
> http://bugzilla.kernel.org/show_bug.cgi?id=14131 ?

I did send a Please-Pull for the cputime branch back in May. Seems like it
never has been pulled. I don't know why, so perhaps I should just retry. 


-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Johan van Baarlen]

Andrew,

with this patch the idle-time in /proc/uptime makes a lot more sense - but
it runs about a factor of 4 too fast (I'm thinking this is not coincidence
- I've got 4 cpu's in this box, and simply adding 4 idle timers means you
are going 4 times too fast).

Can we just add idletime /= (i+1) after the foreachcpu loop, or am I
thinking too easy?


Regards,
Johan



> On Fri, 14 Aug 2009 13:18:08 +0100 (BST) Michael Abbott
> <michael@araneidae.co.uk> wrote:
>
>> Reviving this:
>>
>> On Sat, 9 May 2009, Jan Engelhardt wrote:
>> > starting from v2.6.28-4930-g79741dd lasting thru at least v2.6.29.1,
>> > the second field of /proc/uptime always shows 0.00. This happens for
>> > both the typical i386 (my case) and on an ARM (according to Michael,
>> > cc'ed).
>> >
>> > >From the commit log of 79741dd:
>> >
>> > 	"""The cpu time spent by the idle process actually doing
>> > 	something is currently accounted as idle time. This is plain
>> > 	wrong, the architectures that support VIRT_CPU_ACCOUNTING=y
>> > 	can do better: distinguish between the time spent doing
>> > 	nothing and the time spent by idle doing work. The first is
>> > 	accounted with account_idle_time and the second with
>> > 	account_system_time."""
>> >
>> > Citing Michael from our irc conversation:
>> >
>> > 	"""the writer[committer] [says] that [the] idle process time
>> > 	isn't really idle time ... but that's all that /proc/uptime
>> > 	looks at. I guess fs/proc/uptime.c needs to catch up."""
>> >
>> > So, were the updates to uptime.c missed, or do we now live on with
>> > /proc/uptime constantly having 0?
>>
>> My previous patch seems to have run into the sand.  It every so nearly
>> got
>> pulled into mainstream as far as I can tell, but didn't seem to make it;
>> no idea what happened.
>>
>> So here we go again:
>>
>
> Imagine my surprise to find a version of this patch lurking in Martin's
> tree since June 22.  It's a regression fix!
>
> Johan, does this patch help with the regression you reported in
> http://bugzilla.kernel.org/show_bug.cgi?id=14131

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Martin Schwidefsky]

On Thu, 10 Sep 2009 15:02:53 +0200
"Johan van Baarlen" <vanbaajf@xs4all.nl> wrote:

> with this patch the idle-time in /proc/uptime makes a lot more sense - but
> it runs about a factor of 4 too fast (I'm thinking this is not coincidence
> - I've got 4 cpu's in this box, and simply adding 4 idle timers means you
> are going 4 times too fast).
> 
> Can we just add idletime /= (i+1) after the foreachcpu loop, or am I
> thinking too easy?

With "/= (i+1)" you mean dividing the result by the number of cpus, no?
That doesn't work because of that fact that the value used to contain
the accumulated idle time of a uni-processor system and cpu hotplug.
The only way to get meaningful numbers is to make the value contain the
sum of the idle over all possible cpus. The user space tool that reads
the value needs to take the number of currently active cpus into
account.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] Re: /proc/uptime idle counter remains at 0

[Michael Abbott]

On Thu, 10 Sep 2009, Martin Schwidefsky wrote:
> On Thu, 10 Sep 2009 15:02:53 +0200
> "Johan van Baarlen" <vanbaajf@xs4all.nl> wrote:
> > with this patch the idle-time in /proc/uptime makes a lot more sense - but
> > it runs about a factor of 4 too fast (I'm thinking this is not coincidence
> > - I've got 4 cpu's in this box, and simply adding 4 idle timers means you
> > are going 4 times too fast).
> > 
> > Can we just add idletime /= (i+1) after the foreachcpu loop, or am I
> > thinking too easy?
> 
> With "/= (i+1)" you mean dividing the result by the number of cpus, no? 
> That doesn't work because of that fact that the value used to contain 
> the accumulated idle time of a uni-processor system and cpu hotplug. The 
> only way to get meaningful numbers is to make the value contain the sum 
> of the idle over all possible cpus. The user space tool that reads the 
> value needs to take the number of currently active cpus into account.

I've never liked this solution, as it's hugely unfriendly and requires 
access to detailed information which user space doesn't necessarily have: 
how is any particular user space application supposed to know the detailed 
history of cpu hotplug insertion and removal to accurately compute the 
idle time?

On the other hand, I don't have an alternative to suggest...