Messages in this thread | | | Date | Mon, 17 Aug 2009 16:23:10 +0800 | From | Amerigo Wang <> | Subject | Re: [PATCH] Re: /proc/uptime idle counter remains at 0 |
| |
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.
| |