[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <204f85da5561fcf980a4376f6a611929.squirrel@www.firstfloor.org>
Date: Sun, 21 Nov 2010 18:00:53 +0100
From: "Andi Kleen" <andi@...stfloor.org>
To: "Lin Ming" <ming.m.lin@...el.com>
Cc: "Andi Kleen" <andi@...stfloor.org>,
"Peter Zijlstra" <a.p.zijlstra@...llo.nl>,
"Ingo Molnar" <mingo@...e.hu>,
"Stephane Eranian" <eranian@...gle.com>,
"lkml" <linux-kernel@...r.kernel.org>,
"Frederic Weisbecker" <fweisbec@...il.com>,
"Arjan van de Ven" <arjan@...radead.org>
Subject: Re: [RFC PATCH 2/3 v2] perf: Implement Nehalem uncore pmu
BTW another thing I noticed that if you ever add opcode/address
matching you'll need to add the new parameters for the address
at least to the input perf_event structure. opcode could
in theory be encoded in the upper 32bits like offcore does,
but address needs to be extra. It's only a small
incremental step, but may make this more useful.
>> Really for uncore monitoring there is no need to use an NMI handler.
>> You can't profile a core anyways, so you can just delay the reporting
>> a little bit. It may simplify the code to not use one here
>> and just use an ordinary handler.
>
> OK, I can use on ordinary interrupt handler here.
You'll need to allocate a vector, it shouldn't be too difficult.
>>
>> In general since there is already much trouble with overloaded
>> NMI events avoiding new NMIs is a good idea.
>>
>>
>>
>> > +
>> > +static struct node_hw_events *uncore_events[MAX_NUMNODES];
>>
>> Don't declare static arrays with MAX_NUMNODES, that number can be
>> very large and cause unnecessary bloat. Better use per CPU data or
>> similar
>> (e.g. with alloc_percpu)
>
> I really need is a per physical cpu data here, is alloc_percpu enough?
If you use a per cpu array then each CPU can carry a pointer
to its per socket data structure.
This could use a similar scheme as the per core data I submitted
recently.
>
> Any idea to set this cross-core data?
s/local/atomic/
But if it's just stores/loads without read-modify-write you
can just use normal stores.
>
>>
>> > +static int uncore_pmu_add(struct perf_event *event, int flags)
>> > +{
>> > + int node = numa_node_id();
>>
>> this should be still package id
>
> Understand, this is in my TODO.
With the per cpu pointer scheme you likely don't even need it,
just check the topology at set up time (similar as in my patch,
just using the package)
-Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists