Re: [RFD 1/9] Change cpustat fields to an array.

[Glauber Costa <glommer@parallels.com>]

On 09/27/2011 06:00 PM, Peter Zijlstra wrote:
> On Fri, 2011-09-23 at 19:20 -0300, Glauber Costa wrote:
>>   /* Must have preemption disabled for this to be meaningful. */
>> -#define kstat_this_cpu __get_cpu_var(kstat)
>> +#define kstat_this_cpu this_cpu_ptr(task_group_kstat(current))
>
> This just lost you a debug check, the former would whinge when called
> without preemption, the new one wont. Its part of the this_cpu feature
> set to make debugging impossible.

Why is that?

from percpu.h:
#define __get_cpu_var(var) (*this_cpu_ptr(&(var)))

So I don't get it.

>> +#else
>> +#define kstat_cpu(cpu) per_cpu(kstat, cpu)
>> +#define kstat_this_cpu (&__get_cpu_var(kstat))
>> +#endif
>>
>>   extern unsigned long long nr_context_switches(void);
>>
>> @@ -52,8 +62,8 @@ struct irq_desc;
>>   static inline void kstat_incr_irqs_this_cpu(unsigned int irq,
>>                                              struct irq_desc *desc)
>>   {
>> -       __this_cpu_inc(kstat.irqs[irq]);
>> -       __this_cpu_inc(kstat.irqs_sum);
>> +       kstat_this_cpu->irqs[irq]++;
>> +       kstat_this_cpu->irqs_sum++;
>
> It might be worth looking at the asm output of that, I think you made it
> worse, but I'm not quite sure how smart gcc is, it might just figure out
> what you meant.
>
>>   }

Yes, it is indeed a bit worse, but the reason appears to be an extra 
call to  something related to task_group(). That one is inline, but it 
calls more stuff in the way. And it is hard to dodge from that once we 
move to this path. It might be acceptable to lose some cycles here to 
gain more back once cpuacct is out.

That said, I'll also see if we can squeeze something more clever out of it.