| Message ID | 1424747257-141252-3-git-send-email-david.ahern@oracle.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
Hi Dave: I realize M7 patches are on hold by you. This one (and the first one in this set) has nothing to do with the M7. perf counting mode has been broken for years on sparc; this patch fixes it. David On 2/23/15 8:07 PM, David Ahern wrote: > Currently perf-stat (aka, counting mode) does not work: > > perf stat ./1bi > > Performance counter stats for './1bi': > > 131.864070 task-clock (msec) # 0.993 CPUs utilized > 0 context-switches # 0.000 K/sec > 0 cpu-migrations # 0.000 K/sec > 30 page-faults # 0.228 K/sec > 0 cycles # 0.000 GHz > <not supported> stalled-cycles-frontend:HG > <not supported> stalled-cycles-backend:HG > 0 instructions:HG > 0 branches:HG # 0.000 K/sec > 0 branch-misses:HG # 0.00% of all branches > > 0.132778349 seconds time elapsed > > The reason is that state is never reset (stays with PERF_HES_UPTODATE set). > Add a call to sparc_pmu_enable_event during the added_event handling. > Clean up the encoding since pmu_start calls sparc_pmu_enable_event which > does the same. Passing PERF_EF_RELOAD to sparc_pmu_start means the call > to sparc_perf_event_set_period can be removed as well. > > With this patch: > perf stat ./1bi > > Performance counter stats for './1bi': > > 131.893485 task-clock (msec) # 0.992 CPUs utilized > 0 context-switches # 0.000 K/sec > 0 cpu-migrations # 0.000 K/sec > 30 page-faults # 0.227 K/sec > 546,781,981 cycles # 4.146 GHz > <not supported> stalled-cycles-frontend:HG > <not supported> stalled-cycles-backend:HG > 1,090,528,128 instructions:HG # 1.99 insns per cycle > 10,089,375 branches:HG # 76.496 M/sec > 7,182 branch-misses:HG # 0.07% of all branches > > 0.132918320 seconds time elapsed > > Signed-off-by: David Ahern <david.ahern@oracle.com> > --- > arch/sparc/kernel/perf_event.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/arch/sparc/kernel/perf_event.c b/arch/sparc/kernel/perf_event.c > index 48b565fdb486..86eebfa3b158 100644 > --- a/arch/sparc/kernel/perf_event.c > +++ b/arch/sparc/kernel/perf_event.c > @@ -996,6 +996,8 @@ static void calculate_single_pcr(struct cpu_hw_events *cpuc) > cpuc->pcr[0] |= cpuc->event[0]->hw.config_base; > } > > +static void sparc_pmu_start(struct perf_event *event, int flags); > + > /* On this PMU each PIC has it's own PCR control register. */ > static void calculate_multiple_pcrs(struct cpu_hw_events *cpuc) > { > @@ -1008,20 +1010,13 @@ static void calculate_multiple_pcrs(struct cpu_hw_events *cpuc) > struct perf_event *cp = cpuc->event[i]; > struct hw_perf_event *hwc = &cp->hw; > int idx = hwc->idx; > - u64 enc; > > if (cpuc->current_idx[i] != PIC_NO_INDEX) > continue; > > - sparc_perf_event_set_period(cp, hwc, idx); > cpuc->current_idx[i] = idx; > > - enc = perf_event_get_enc(cpuc->events[i]); > - cpuc->pcr[idx] &= ~mask_for_index(idx); > - if (hwc->state & PERF_HES_STOPPED) > - cpuc->pcr[idx] |= nop_for_index(idx); > - else > - cpuc->pcr[idx] |= event_encoding(enc, idx); > + sparc_pmu_start(cp, PERF_EF_RELOAD); > } > out: > for (i = 0; i < cpuc->n_events; i++) { > -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: David Ahern <david.ahern@oracle.com> Date: Thu, 19 Mar 2015 08:15:17 -0600 > I realize M7 patches are on hold by you. This one (and the first one > in this set) has nothing to do with the M7. perf counting mode has > been broken for years on sparc; this patch fixes it. Not on hold, I'm simply waiting for them to be resubmitted. Anything not marked "under review" in patchwork needs to be explicitly resubmitting anew if you want me to consider it. Sorry if that wasn't clear. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/19/15 12:29 PM, David Miller wrote: > From: David Ahern <david.ahern@oracle.com> > Date: Thu, 19 Mar 2015 08:15:17 -0600 > >> I realize M7 patches are on hold by you. This one (and the first one >> in this set) has nothing to do with the M7. perf counting mode has >> been broken for years on sparc; this patch fixes it. > > Not on hold, I'm simply waiting for them to be resubmitted. > > Anything not marked "under review" in patchwork needs to be explicitly > resubmitting anew if you want me to consider it. > > Sorry if that wasn't clear. > Help me understand your process for patches ... I sent the patches to you and the mailing list. You responded only to the M7 patch (2 of 3). Should I take that to mean the entire set is invalidated? ie., how do I know if a patch is marked 'under review'? If patches are not directly related to one another you want them as individual patches -- versus what I did here which is 3 individual patches that touch sparc/perf code. Most of my patches have gone through acme and he gladly takes individual patches from a set. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: David Ahern <david.ahern@oracle.com> Date: Thu, 19 Mar 2015 12:34:44 -0600 > how do I know if a patch is marked 'under review'? Ummm, you look at your patches in patchwork? You can use state "any" if you don't know the disposition. http://patchwork.ozlabs.org/project/sparclinux/list/?state=* -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/19/15 12:44 PM, David Miller wrote: > From: David Ahern <david.ahern@oracle.com> > Date: Thu, 19 Mar 2015 12:34:44 -0600 > >> how do I know if a patch is marked 'under review'? > > Ummm, you look at your patches in patchwork? > > You can use state "any" if you don't know the disposition. > > http://patchwork.ozlabs.org/project/sparclinux/list/?state=* > Sowmini just pointed out that site. Was not aware of it before. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sparc/kernel/perf_event.c b/arch/sparc/kernel/perf_event.c index 48b565fdb486..86eebfa3b158 100644 --- a/arch/sparc/kernel/perf_event.c +++ b/arch/sparc/kernel/perf_event.c @@ -996,6 +996,8 @@ static void calculate_single_pcr(struct cpu_hw_events *cpuc) cpuc->pcr[0] |= cpuc->event[0]->hw.config_base; } +static void sparc_pmu_start(struct perf_event *event, int flags); + /* On this PMU each PIC has it's own PCR control register. */ static void calculate_multiple_pcrs(struct cpu_hw_events *cpuc) { @@ -1008,20 +1010,13 @@ static void calculate_multiple_pcrs(struct cpu_hw_events *cpuc) struct perf_event *cp = cpuc->event[i]; struct hw_perf_event *hwc = &cp->hw; int idx = hwc->idx; - u64 enc; if (cpuc->current_idx[i] != PIC_NO_INDEX) continue; - sparc_perf_event_set_period(cp, hwc, idx); cpuc->current_idx[i] = idx; - enc = perf_event_get_enc(cpuc->events[i]); - cpuc->pcr[idx] &= ~mask_for_index(idx); - if (hwc->state & PERF_HES_STOPPED) - cpuc->pcr[idx] |= nop_for_index(idx); - else - cpuc->pcr[idx] |= event_encoding(enc, idx); + sparc_pmu_start(cp, PERF_EF_RELOAD); } out: for (i = 0; i < cpuc->n_events; i++) {
Currently perf-stat (aka, counting mode) does not work: perf stat ./1bi Performance counter stats for './1bi': 131.864070 task-clock (msec) # 0.993 CPUs utilized 0 context-switches # 0.000 K/sec 0 cpu-migrations # 0.000 K/sec 30 page-faults # 0.228 K/sec 0 cycles # 0.000 GHz <not supported> stalled-cycles-frontend:HG <not supported> stalled-cycles-backend:HG 0 instructions:HG 0 branches:HG # 0.000 K/sec 0 branch-misses:HG # 0.00% of all branches 0.132778349 seconds time elapsed The reason is that state is never reset (stays with PERF_HES_UPTODATE set). Add a call to sparc_pmu_enable_event during the added_event handling. Clean up the encoding since pmu_start calls sparc_pmu_enable_event which does the same. Passing PERF_EF_RELOAD to sparc_pmu_start means the call to sparc_perf_event_set_period can be removed as well. With this patch: perf stat ./1bi Performance counter stats for './1bi': 131.893485 task-clock (msec) # 0.992 CPUs utilized 0 context-switches # 0.000 K/sec 0 cpu-migrations # 0.000 K/sec 30 page-faults # 0.227 K/sec 546,781,981 cycles # 4.146 GHz <not supported> stalled-cycles-frontend:HG <not supported> stalled-cycles-backend:HG 1,090,528,128 instructions:HG # 1.99 insns per cycle 10,089,375 branches:HG # 76.496 M/sec 7,182 branch-misses:HG # 0.07% of all branches 0.132918320 seconds time elapsed Signed-off-by: David Ahern <david.ahern@oracle.com> --- arch/sparc/kernel/perf_event.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)