Hi All,
What do people think of a patch to change the fields in cpu_usage_stat
from unsigned ints to unsigned long longs? And the same change for
nr_switches in the runqueue structure too?
My colleagues and I believe this is necessary given that the accrued
usage for one CPU will overflow 32 bits fairly quickly if its
continuously busy. If I understand it correctly, for HZ being 100,
overflow can occur in 497 days. Its actually worse for context
switches on a busy system, for we've been seeing an average of ten
switches a tick for some of the statistics we have.
I haven't made any changes to the interrupt statistics to use unisnged
long long however. Perhaps that should be considered as well, but
that'll mean changes to architecture dependent code...
--
Kingsley
Kingsley Cheung <[email protected]> wrote:
>
> Hi All,
>
> What do people think of a patch to change the fields in cpu_usage_stat
> from unsigned ints to unsigned long longs? And the same change for
> nr_switches in the runqueue structure too?
Sounds unavoidable.
> Its actually worse for context
> switches on a busy system, for we've been seeing an average of ten
> switches a tick for some of the statistics we have.
Sounds broken. What CPU scheduler are you using?
> for_each_online_cpu(i) {
> - seq_printf(p, "cpu%d %u %u %u %u %u %u %u\n",
> + seq_printf(p, "cpu%d %llu %llu %llu %llu %llu %llu %llu\n",
> i,
> - jiffies_to_clock_t(kstat_cpu(i).cpustat.user),
> - jiffies_to_clock_t(kstat_cpu(i).cpustat.nice),
> - jiffies_to_clock_t(kstat_cpu(i).cpustat.system),
> - jiffies_to_clock_t(kstat_cpu(i).cpustat.idle),
> - jiffies_to_clock_t(kstat_cpu(i).cpustat.iowait),
> - jiffies_to_clock_t(kstat_cpu(i).cpustat.irq),
> - jiffies_to_clock_t(kstat_cpu(i).cpustat.softirq));
> + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.user),
> + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.nice),
> + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.system),
> + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.idle),
> + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.iowait),
> + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.irq),
> + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.softirq));
jiffies_64_to_clock_t() takes and returns a u64, not an unsigned long long.
> }
> - seq_printf(p, "intr %u", sum);
> + seq_printf(p, "intr %llu", sum);
It would be best to convert everything to u64, not to unsigned long long.
But cast them to unsigned long long for printk.
It's a bit ugly, but at least it pins everything down to know types and
sizes on all architectures.
> struct cpu_usage_stat {
> - unsigned int user;
> - unsigned int nice;
> - unsigned int system;
> - unsigned int softirq;
> - unsigned int irq;
> - unsigned int idle;
> - unsigned int iowait;
> + unsigned long long user;
> + unsigned long long nice;
> + unsigned long long system;
> + unsigned long long softirq;
> + unsigned long long irq;
> + unsigned long long idle;
> + unsigned long long iowait;
Do these have appropriate locking or are we just accepting the occasional
glitch?
On Mon, Mar 08, 2004 at 06:53:54PM -0800, Andrew Morton wrote:
> Kingsley Cheung <[email protected]> wrote:
> >
> > Hi All,
> >
> > What do people think of a patch to change the fields in cpu_usage_stat
> > from unsigned ints to unsigned long longs? And the same change for
> > nr_switches in the runqueue structure too?
>
> Sounds unavoidable.
>
> > Its actually worse for context
> > switches on a busy system, for we've been seeing an average of ten
> > switches a tick for some of the statistics we have.
>
> Sounds broken. What CPU scheduler are you using?
Um, what do you mean by broken?
Well, as for the scheduler, its the Entitlement Based Scheduler, but
it doesn't look like its got anything to do with the scheduler. Some
work loads we have been testing just have processes that come and go
so frequently that the context switch rate is high. Even when Ingo
posted his original O(1) patch (see
http://marc.theaimsgroup.com/?l=linux-kernel&m=101010394225604&w=2) he
claimed high context switch rates.
>
> > for_each_online_cpu(i) {
> > - seq_printf(p, "cpu%d %u %u %u %u %u %u %u\n",
> > + seq_printf(p, "cpu%d %llu %llu %llu %llu %llu %llu %llu\n",
> > i,
> > - jiffies_to_clock_t(kstat_cpu(i).cpustat.user),
> > - jiffies_to_clock_t(kstat_cpu(i).cpustat.nice),
> > - jiffies_to_clock_t(kstat_cpu(i).cpustat.system),
> > - jiffies_to_clock_t(kstat_cpu(i).cpustat.idle),
> > - jiffies_to_clock_t(kstat_cpu(i).cpustat.iowait),
> > - jiffies_to_clock_t(kstat_cpu(i).cpustat.irq),
> > - jiffies_to_clock_t(kstat_cpu(i).cpustat.softirq));
> > + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.user),
> > + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.nice),
> > + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.system),
> > + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.idle),
> > + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.iowait),
> > + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.irq),
> > + jiffies_64_to_clock_t(kstat_cpu(i).cpustat.softirq));
>
> jiffies_64_to_clock_t() takes and returns a u64, not an unsigned long long.
>
> > }
> > - seq_printf(p, "intr %u", sum);
> > + seq_printf(p, "intr %llu", sum);
>
> It would be best to convert everything to u64, not to unsigned long long.
> But cast them to unsigned long long for printk.
Makes sense given the way jiffies_64_to_clock_t() works. The casts
are not needed right? IMHO I would have thought unsigned long long is
guarenteed to be at least 64 bits by the C99 standard.
Anyway I've modified the patch to make cpu_usage_stat use u64. I
didn't change nr_switches() however...
>
> It's a bit ugly, but at least it pins everything down to know types and
> sizes on all architectures.
>
> > struct cpu_usage_stat {
> > - unsigned int user;
> > - unsigned int nice;
> > - unsigned int system;
> > - unsigned int softirq;
> > - unsigned int irq;
> > - unsigned int idle;
> > - unsigned int iowait;
> > + unsigned long long user;
> > + unsigned long long nice;
> > + unsigned long long system;
> > + unsigned long long softirq;
> > + unsigned long long irq;
> > + unsigned long long idle;
> > + unsigned long long iowait;
>
> Do these have appropriate locking or are we just accepting the occasional
> glitch?
Yeah, the occasional glitch shouldn't be a problem ;)
--
Kingsley
On Tue, Mar 09, 2004 at 04:57:04PM +1100, Kingsley Cheung wrote:
> On Mon, Mar 08, 2004 at 06:53:54PM -0800, Andrew Morton wrote:
> > Kingsley Cheung <[email protected]> wrote:
> > >
> > > Hi All,
> > >
> > > What do people think of a patch to change the fields in cpu_usage_stat
> > > from unsigned ints to unsigned long longs? And the same change for
> > > nr_switches in the runqueue structure too?
> >
> > Sounds unavoidable.
> >
> > > Its actually worse for context
> > > switches on a busy system, for we've been seeing an average of ten
> > > switches a tick for some of the statistics we have.
> >
> > Sounds broken. What CPU scheduler are you using?
>
> Um, what do you mean by broken?
>
> Well, as for the scheduler, its the Entitlement Based Scheduler, but
> it doesn't look like its got anything to do with the scheduler. Some
> work loads we have been testing just have processes that come and go
> so frequently that the context switch rate is high. Even when Ingo
> posted his original O(1) patch (see
> http://marc.theaimsgroup.com/?l=linux-kernel&m=101010394225604&w=2) he
> claimed high context switch rates.
Oh, just in case there's some confusion... even though we've been
seeing it for EBS, the patch is for 2.6.3.
--
Kingsley