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

From: Glauber Costa
Date: Wed Sep 28 2011 - 11:14:26 EST


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.



--
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/