Re: [PATCH][3/7] perfctr-2.7.2 for 2.6.6-mm2: x86_64

From: Andi Kleen
Date: Fri May 14 2004 - 10:21:25 EST


Mikael Pettersson <mikpe@xxxxxxxxx> writes:

Before merging all that I would definitely recommend some generic
module to allocate performance counters. IBM had a patch for this
long ago, and it is even more needed now.

> diff -ruN linux-2.6.6-mm2/drivers/perfctr/x86_64.c linux-2.6.6-mm2.perfctr-2.7.2.x86_64/drivers/perfctr/x86_64.c
> --- linux-2.6.6-mm2/drivers/perfctr/x86_64.c 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.6-mm2.perfctr-2.7.2.x86_64/drivers/perfctr/x86_64.c 2004-05-14 14:45:43.990230001 +0200
> @@ -0,0 +1,660 @@
> +/* $Id: x86_64.c,v 1.27 2004/05/13 23:32:50 mikpe Exp $
> + * x86_64 performance-monitoring counters driver.

[...]

Can't you share most/all of that file with i386 ?
You'll want that definitely once you support Intel CPUs too,
and you have to do that eventually.

Same for include/asm-x86_64/perfctr.h

+struct per_cpu_cache { /* roughly a subset of perfctr_cpu_state */
+ union {
+ unsigned int id; /* cache owner id */
+ } k1;
+ struct {
+ /* NOTE: these caches have physical indices, not virtual */
+ unsigned int evntsel[4];
+ } control;
+} ____cacheline_aligned;
+static struct per_cpu_cache per_cpu_cache[NR_CPUS] __cacheline_aligned;

This should use per_cpu_data

+static unsigned int new_id(void)

[...]

Why can't that wrap? Maybe it should use the functions in lib/idr.c ?

+ if( perfctr_cstatus_has_tsc(cstatus) )
+ rdtscl(ctrs->tsc);
+ nrctrs = perfctr_cstatus_nractrs(cstatus);
+ for(i = 0; i < nrctrs; ++i) {
+ unsigned int pmc = state->pmc[i].map;
+ rdpmc_low(pmc, ctrs->pmc[i]);
+ }

K8 has speculative rdtsc. Most likely you want a sync_core() somewhere
in there.

The way you set your brackets is weird.

Why do you check for K8 C stepping? I don't see any code that
does anything special with that.

-Andi

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