2009-09-24 11:31:38

by Martin Schwidefsky

[permalink] [raw]
Subject: [GIT PULL] cputime tree

Hi Linus,

with git commit 79741dd "idle cputime accounting" the semantic
of the stime field of the idle processes has changed. It used to
contain the amount of time the idle process has been scheduled.
Since git commit 79741dd is contains the cpu time spent in the
system by the idle process.

This change broke the output of second field of /proc/uptime. On
systems without VIRT_CPU_ACCOUNTING the field is always zero.
The legacy output of the field is the amount of time the idle
process has been scheduled on cpu #0. It is good enough to
calculate the load on an uni-processor system, it is useless
on a multi-processor. To restore the legacy behaviour and give
meaning to the multi-processor case the best we could come up
with is to add the idle time over all cpus. That fixes uni-
processors systems and gives a defined semantic on smp.

So please pull from 'cputime' branch of

git://git390.marist.edu/pub/scm/linux-2.6.git cputime

to receive the following updates:

Michael Abbott (1):
Fix idle time field in /proc/uptime

fs/proc/uptime.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)

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);


2009-09-24 16:52:39

by Andrew Morton

[permalink] [raw]
Subject: Re: [GIT PULL] cputime tree

On Thu, 24 Sep 2009 13:31:36 +0200 Martin Schwidefsky <[email protected]> wrote:

> Hi Linus,
>
> with git commit 79741dd "idle cputime accounting" the semantic
> of the stime field of the idle processes has changed. It used to
> contain the amount of time the idle process has been scheduled.
> Since git commit 79741dd is contains the cpu time spent in the
> system by the idle process.
>
> This change broke the output of second field of /proc/uptime. On
> systems without VIRT_CPU_ACCOUNTING the field is always zero.
> The legacy output of the field is the amount of time the idle
> process has been scheduled on cpu #0. It is good enough to
> calculate the load on an uni-processor system, it is useless
> on a multi-processor. To restore the legacy behaviour and give
> meaning to the multi-processor case the best we could come up
> with is to add the idle time over all cpus. That fixes uni-
> processors systems and gives a defined semantic on smp.
>
> So please pull from 'cputime' branch of
>
> git://git390.marist.edu/pub/scm/linux-2.6.git cputime
>
> to receive the following updates:
>
> Michael Abbott (1):
> Fix idle time field in /proc/uptime
>
> fs/proc/uptime.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> 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);

This is a regression fix, iirc? One which is applicable for several 2.6.x
kernel versions?

2009-09-24 19:49:15

by Michael Abbott

[permalink] [raw]
Subject: Re: [GIT PULL] cputime tree

On Thu, 24 Sep 2009, Andrew Morton wrote:
> On Thu, 24 Sep 2009 13:31:36 +0200 Martin Schwidefsky <[email protected]> wrote:
>
> > Hi Linus,
> >
> > with git commit 79741dd "idle cputime accounting" the semantic
> > of the stime field of the idle processes has changed. It used to
> > contain the amount of time the idle process has been scheduled.
> > Since git commit 79741dd is contains the cpu time spent in the
> > system by the idle process.
> >
> > This change broke the output of second field of /proc/uptime. On
> > systems without VIRT_CPU_ACCOUNTING the field is always zero.
> > The legacy output of the field is the amount of time the idle
> > process has been scheduled on cpu #0. It is good enough to
> > calculate the load on an uni-processor system, it is useless
> > on a multi-processor. To restore the legacy behaviour and give
> > meaning to the multi-processor case the best we could come up
> > with is to add the idle time over all cpus. That fixes uni-
> > processors systems and gives a defined semantic on smp.
> >
> > So please pull from 'cputime' branch of
> >
> > git://git390.marist.edu/pub/scm/linux-2.6.git cputime
> >
> > to receive the following updates:
> >
> > Michael Abbott (1):
> > Fix idle time field in /proc/uptime
> >
> > fs/proc/uptime.c | 7 ++++++-
> > 1 files changed, 6 insertions(+), 1 deletions(-)
> >
> > 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);
>
> This is a regression fix, iirc? One which is applicable for several
> 2.6.x kernel versions?

Yes indeed, all since the regression was introduced I think around about
.28 (though I seem to remember having to tweak the code a little when
rebasing it forward).

2009-10-01 22:49:22

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [GIT PULL] cputime tree

On Thu, Sep 24, 2009 at 08:49:10PM +0100, Michael Abbott wrote:
> On Thu, 24 Sep 2009, Andrew Morton wrote:
> > On Thu, 24 Sep 2009 13:31:36 +0200 Martin Schwidefsky <[email protected]> wrote:
> >
> > > Hi Linus,
> > >
> > > with git commit 79741dd "idle cputime accounting" the semantic
> > > of the stime field of the idle processes has changed. It used to
> > > contain the amount of time the idle process has been scheduled.
> > > Since git commit 79741dd is contains the cpu time spent in the
> > > system by the idle process.
> > >
> > > This change broke the output of second field of /proc/uptime. On
> > > systems without VIRT_CPU_ACCOUNTING the field is always zero.
> > > The legacy output of the field is the amount of time the idle
> > > process has been scheduled on cpu #0. It is good enough to
> > > calculate the load on an uni-processor system, it is useless
> > > on a multi-processor. To restore the legacy behaviour and give
> > > meaning to the multi-processor case the best we could come up
> > > with is to add the idle time over all cpus. That fixes uni-
> > > processors systems and gives a defined semantic on smp.
> > >
> > > So please pull from 'cputime' branch of
> > >
> > > git://git390.marist.edu/pub/scm/linux-2.6.git cputime
> > >
> > > to receive the following updates:
> > >
> > > Michael Abbott (1):
> > > Fix idle time field in /proc/uptime
> > >
> > > fs/proc/uptime.c | 7 ++++++-
> > > 1 files changed, 6 insertions(+), 1 deletions(-)
> > >
> > > 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);
> >
> > This is a regression fix, iirc? One which is applicable for several
> > 2.6.x kernel versions?
>
> Yes indeed, all since the regression was introduced I think around about
> .28 (though I seem to remember having to tweak the code a little when
> rebasing it forward).

Thanks, I've queued this up for .30 and .31 stable kernels.

greg k-h