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

Cyrill Gorcunov
2010-02-10T10:38:14Z

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);
>
>
>