[#5469] CPU hardly idles anymore in svn trunk
Submitted By: Mingquan Pan
Open Date
2009-08-27 01:36:14 Close Date
2009-10-15 22:53:02
Priority:
Medium Assignee:
Barry Song
Status:
Closed Fixed In Release:
N/A
Found In Release:
2010R1 Release:
Category:
N/A Board:
N/A
Processor:
BF537 Silicon Revision:
Is this bug repeatable?:
Yes Resolution:
Upstream
Uboot version or rev.:
Toolchain version or rev.:
09r1-rc9
App binary format:
N/A
Summary: CPU hardly idles anymore in svn trunk
Details:
cat /proc/uptime always shows 0 for the second param on trunk head, while uptime looks good.
root:/> uptime
04:08:14 up 1 min, load average: 0.00, 0.00, 0.00
root:/> cat /proc/uptime
108.52 0.00
root:/> cat /proc/uptime
111.44 0.00
root:/> uptime
04:08:22 up 1 min, load average: 0.00, 0.00, 0.00
root:/> cat /proc/uptime
153.02 0.00
root:/> version
kernel: Linux release 2.6.30.5-ADI-2010R1-pre-svn7234, build #4 Thu Aug 27 11:36:22 CST 2009
toolchain: bfin-uclinux-gcc release gcc version 4.1.2 (ADI svn)
user-dist: release svn-8770, build #35 Thu Aug 27 11:32:54 CST 2009
Follow-ups
--- Mike Frysinger 2009-08-27 06:50:35
like the documentation says:
/proc/uptime
This file contains two numbers: the uptime of the system (seconds), and
the amount of time spent in idle process (seconds).
the 2nd field is idle time, so the output of `uptime` is irrelevant. if the #
is to be believed, the processor is hardly ever idling. which is a different
problem of sorts.
--- Barry Song 2009-09-04 06:08:12
Let me check whether it is related with this patch:
[PATCH] Re: /proc/uptime idle counter remains at 0
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;
}
--- Barry Song 2009-09-06 23:07:16
Yes. This patch can fix the problem. For example:
root:/> uname -a
Linux blackfin 2.6.30.5-ADI-2010R1-pre-svn7261 #823 Mon Sep 7 11:01:28 CST 2009
blackfin GNU/Linux
root:/> cat /proc/uptime
139.73 138.44
root:/> cat /proc/uptime
140.85 139.55
root:/> cat /proc/uptime
142.15 140.84
root:/> cat /proc/uptime
143.23 141.92
root:/> cat /proc/uptime
148.21 146.89
root:/>
But the patch is not in Linus' kernel tree until now. Let's wait it to be
applied in the tree and let Mike to upgrade.
--------------------------------------
LKML:
Amerigo Wang
to Michael, Jan, Martin, Linux
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.
--- Mingquan Pan 2009-10-15 22:53:02
Yes, cat /proc/uptime is good now. Close.
--- Mike Frysinger 2009-10-15 23:28:27
it was merged into mainline and was part of 2.6.31.2
Files
Changes
Commits
Dependencies
Duplicates
Associations
Tags
File Name File Type File Size Posted By
No Files Were Found