On Fri, 14 Aug 2009 13:18:08 +0100 (BST) Michael Abbott <[email protected]> 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 <[email protected]>
AuthorDate: Mon Jun 22 12:19:25 2009 +0200
Commit: Martin Schwidefsky <[email protected]>
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 <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
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);
On Tue, 8 Sep 2009 22:58:58 -0700
Andrew Morton <[email protected]> wrote:
> On Fri, 14 Aug 2009 13:18:08 +0100 (BST) Michael Abbott <[email protected]> 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.
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
> <[email protected]> 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
On Thu, 10 Sep 2009 15:02:53 +0200
"Johan van Baarlen" <[email protected]> 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.
On Thu, 10 Sep 2009, Martin Schwidefsky wrote:
> On Thu, 10 Sep 2009 15:02:53 +0200
> "Johan van Baarlen" <[email protected]> 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...