On Mon, Mar 08, 2004 at 06:53:54PM -0800, Andrew Morton wrote:
> Kingsley Cheung <kingsley@aurema.com> 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