Re: [RFC perf,x86] P4 PMU early draft

[Cyrill Gorcunov <gorcunov@gmail.com>]

On 2/10/10, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, 2010-02-10 at 01:39 +0300, Cyrill Gorcunov wrote:
>
>
>> Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event.c
>> =====================================================================
>> --- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event.c
>> +++ linux-2.6.git/arch/x86/kernel/cpu/perf_event.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/bitops.h>
>>
>>  #include <asm/apic.h>
>> +#include <asm/perf_p4.h>
>>  #include <asm/stacktrace.h>
>>  #include <asm/nmi.h>
>>
>> @@ -140,6 +141,7 @@ struct x86_pmu {
>>  	u64		max_period;
>>  	u64		intel_ctrl;
>>  	int		(*hw_config)(struct perf_event_attr *attr, struct hw_perf_event
>> *hwc);
>> +	int		(*schedule_events)(struct cpu_hw_events *cpuc, int n, int *assign,
>> int cpu);
>>  	void		(*enable_bts)(u64 config);
>>  	void		(*disable_bts)(void);
>>
>
>> +/*
>> + * This is the most important routine of Netburst PMU actually
>> + * and need a huge speedup!
>> + */
>> +static int p4_pmu_schedule_events(struct cpu_hw_events *cpuc, int n, int
>> *assign, int cpu)
>> +{
>
>> +}
>
>
>> -static int x86_schedule_events(struct cpu_hw_events *cpuc, int n, int
>> *assign)
>> +/* we don't use cpu argument here at all */
>> +static int x86_schedule_events(struct cpu_hw_events *cpuc, int n, int
>> *assign, int cpu)
>>  {
>>  	struct event_constraint *c, *constraints[X86_PMC_IDX_MAX];
>>  	unsigned long used_mask[BITS_TO_LONGS(X86_PMC_IDX_MAX)];
>
>
>> @@ -1796,7 +2305,7 @@ static int x86_pmu_enable(struct perf_ev
>>  	if (n < 0)
>>  		return n;
>>
>> -	ret = x86_schedule_events(cpuc, n, assign);
>> +	ret = x86_pmu.schedule_events(cpuc, n, assign, 0);
>>  	if (ret)
>>  		return ret;
>>  	/*
>
> This look like a bug, surely we can run on !cpu0.

Definitely! Thanks! It should be smp_processor_id()

>
>> @@ -2313,7 +2822,7 @@ int hw_perf_group_sched_in(struct perf_e
>>  	if (n0 < 0)
>>  		return n0;
>>
>> -	ret = x86_schedule_events(cpuc, n0, assign);
>> +	ret = x86_pmu.schedule_events(cpuc, n0, assign, cpu);
>>  	if (ret)
>>  		return ret;
>>
>
> I'd try BUG_ON(cpu != smp_processor_id()) and scrap passing that cpu
> thing around.
>

no, i need cpu to find out if event has migrated from other thread and
then i switch
some thread dependant flags in hw::config (ie escr and cccr), or i
miss something and events in one cpu just can't migrate to another
cpu?

>> @@ -2700,6 +3232,7 @@ static int validate_group(struct perf_ev
>>  {
>>  	struct perf_event *leader = event->group_leader;
>>  	struct cpu_hw_events *fake_cpuc;
>> +	int cpu = smp_processor_id();
>>  	int ret, n;
>>
>>  	ret = -ENOMEM;
>> @@ -2725,7 +3258,7 @@ static int validate_group(struct perf_ev
>>
>>  	fake_cpuc->n_events = n;
>>
>> -	ret = x86_schedule_events(fake_cpuc, n, NULL);
>> +	ret = x86_pmu.schedule_events(fake_cpuc, n, NULL, cpu);
>>
>>  out_free:
>>  	kfree(fake_cpuc);
>
>
>