Re: [PATCH] For preventing kstat overflow

[Andrew Morton]

Kingsley Cheung <kingsley@xxxxxxxxxx> 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?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/