Re: Performance Events hangs with Intel P4 system

Jaswinder Singh Rajput
2010-05-14T16:22:34Z

Hello Cyrill,

On Fri, May 14, 2010 at 8:22 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> [PATCH -tip/master] x86,perf: P4 PMU - fix counters allocation logic and sign issue
>
> Jaswinder reported GP:
> |
> | Message from syslogd@ht at May 14 09:39:32 ...
> | kernel:[  314.908612] EIP: [<c100ccca>]
> | x86_perf_event_set_period+0x19d/0x1b2 SS:ESP 0068:edac3d70
> |
>
> Ming has narrowed it down to comparision issue between arguments with
> different sizes. As result event index reaches value 255 which in turn
> leads to GP fault.
>
> Also it was found that p4_next_cntr has a broken logic and should return
> counter index if only it was not yet borrowed for another event.
>
> Reported-by: Jaswinder Singh Rajput <jaswinderlinux@gmail.com>
> Reported-by: Lin Ming <ming.m.lin@intel.com>
> Bisected-by: Lin Ming <ming.m.lin@intel.com>
> CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> CC: Ingo Molnar <mingo@elte.hu>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> ---

Yes, this works for me. Now I am not getting general protection fault.

It seems hardware events are not supported for P4 yet.

$ ./perf stat -e
cycles,instructions,cache-references,cache-misses,branches,branch-misses,bus-cycles
ls > /dev/null

Performance counter stats for 'ls':

<not counted> cycles
601636 instructions # 0.000 IPC (scaled
from 91.80%)
<not counted> cache-references
<not counted> cache-misses
<not counted> branches
<not counted> branch-misses
<not counted> bus-cycles

0.003364910 seconds time elapsed

Thanks,
--
Jaswinder Singh.

>  arch/x86/kernel/cpu/perf_event_p4.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
> =====================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event_p4.c
> +++ linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
> @@ -747,11 +747,11 @@ static int p4_get_escr_idx(unsigned int
>  static int p4_next_cntr(int thread, unsigned long *used_mask,
>                        struct p4_event_bind *bind)
>  {
> -       int i = 0, j;
> +       int i, j;
>
>        for (i = 0; i < P4_CNTR_LIMIT; i++) {
> -               j = bind->cntr[thread][i++];
> -               if (j == -1 || !test_bit(j, used_mask))
> +               j = bind->cntr[thread][i];
> +               if (j != (unsigned char)-1 && !test_bit(j, used_mask))
>                        return j;
>        }
>
>