All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] MIPS: perf: MT fixes and improvements
@ 2018-04-03 12:31 ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Maciej W. Rozycki,
	Peter Zijlstra, linux-kernel, Paul Burton, Ingo Molnar,
	Jiri Olsa, Alexander Shishkin, Arnaldo Carvalho de Melo

This series addresses a few issues with how the MIPS performance
counters code supports the hardware multithreading MT ASE.
Firstly, implementations of the MT ASE may implement performance counters
per core or per thread(TC). MIPS Techologies & BMIPS5000 implementations
signal this via a bit in the implmentation specific CONFIG7 register.
Since this register is implementation specific, checking it should be
guarded by a PRID check. This also replaces a bit defined by a magic
number.

Secondly, the code currently uses vpe_id(), defined as
smp_processor_id(), divided by 2, to share core performance counters
between VPEs. This relies on a couple of assumptions of the hardware
implementation to function correctly (always 2 VPEs per core, and the
hardware reading only the least significant bit).

Finally, the method of sharing core performance counters between VPEs is
suboptimal since it does not allow one process running on a VPE to use
all of the performance counters available to it, because the kernel will
reserve half of the coutners for the other VPE even if it may never use
them. This reservation at counter probe is replaced with an allocation
on use strategy.

Tested on a MIPS Creator CI40 (2C2T MIPS InterAptiv with per-TC
counters, though for the purposes of testing the per-TC availability was
hardcoded to allow testing both paths).

Series applies to v4.16-rc7



Matt Redfearn (5):
  MIPS: perf: More robustly probe for the presence of per-tc counters
  MIPS: perf: Use correct VPE ID when setting up VPE tracing
  MIPS: perf: Fix perf with MT counting other threads
  MIPS: perf: Allocate per-core counters on demand
  MIPS: perf: Fold vpe_id() macro into it's one last usage

 arch/mips/include/asm/mipsregs.h     |  10 ++
 arch/mips/kernel/perf_event_mipsxx.c | 257 +++++++++++++++++++++--------------
 2 files changed, 162 insertions(+), 105 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/5] MIPS: perf: MT fixes and improvements
@ 2018-04-03 12:31 ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Maciej W. Rozycki,
	Peter Zijlstra, linux-kernel, Paul Burton, Ingo Molnar,
	Jiri Olsa, Alexander Shishkin, Arnaldo Carvalho de Melo

This series addresses a few issues with how the MIPS performance
counters code supports the hardware multithreading MT ASE.
Firstly, implementations of the MT ASE may implement performance counters
per core or per thread(TC). MIPS Techologies & BMIPS5000 implementations
signal this via a bit in the implmentation specific CONFIG7 register.
Since this register is implementation specific, checking it should be
guarded by a PRID check. This also replaces a bit defined by a magic
number.

Secondly, the code currently uses vpe_id(), defined as
smp_processor_id(), divided by 2, to share core performance counters
between VPEs. This relies on a couple of assumptions of the hardware
implementation to function correctly (always 2 VPEs per core, and the
hardware reading only the least significant bit).

Finally, the method of sharing core performance counters between VPEs is
suboptimal since it does not allow one process running on a VPE to use
all of the performance counters available to it, because the kernel will
reserve half of the coutners for the other VPE even if it may never use
them. This reservation at counter probe is replaced with an allocation
on use strategy.

Tested on a MIPS Creator CI40 (2C2T MIPS InterAptiv with per-TC
counters, though for the purposes of testing the per-TC availability was
hardcoded to allow testing both paths).

Series applies to v4.16-rc7



Matt Redfearn (5):
  MIPS: perf: More robustly probe for the presence of per-tc counters
  MIPS: perf: Use correct VPE ID when setting up VPE tracing
  MIPS: perf: Fix perf with MT counting other threads
  MIPS: perf: Allocate per-core counters on demand
  MIPS: perf: Fold vpe_id() macro into it's one last usage

 arch/mips/include/asm/mipsregs.h     |  10 ++
 arch/mips/kernel/perf_event_mipsxx.c | 257 +++++++++++++++++++++--------------
 2 files changed, 162 insertions(+), 105 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/5] MIPS: perf: More robustly probe for the presence of per-tc counters
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Maciej W. Rozycki,
	Peter Zijlstra, linux-kernel, Paul Burton, Ingo Molnar,
	Jiri Olsa, Alexander Shishkin, Arnaldo Carvalho de Melo

Processors implementing the MIPS MT ASE may have performance counters
implemented per core or per TC. Processors implemented by MIPS and the
BMIPS5000 signify presence per TC through a bit in the implementation
specific Config7 register. Currently the code which probes for their
presence blindly reads a magic number corresponding to this bit, despite
it potentially having a different meaning in the CPU implementation.

Fix this by introducing probe_mipsmt_pertccounters() to probe for their
presence. This detects the ases implemented in the CPU, and reads any
implementation specific bit flagging their presence. In the case of MIPS
and BMIPS5000 implementations, this bit is Config7.PTC. A definition of
this bit is added in mipsregs.h for both MIPS Technologies
implementations and BMIPS5000.

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

The test of Config7.PTC was previously enabled when CONFIG_BMIPS5000 was
enabled, whereas now it is based on Broadcom PRID. If this bit is not
present / defined for all Broadcom implementations, please could someone
present a correct alternative means to detect it?

---
 arch/mips/include/asm/mipsregs.h     | 10 ++++++++++
 arch/mips/kernel/perf_event_mipsxx.c | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 858752dac337..4f61e16f43c3 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -684,6 +684,16 @@
 #define MIPS_CONF7_IAR		(_ULCAST_(1) << 10)
 #define MIPS_CONF7_AR		(_ULCAST_(1) << 16)
 
+/* Config7 Bits specific to MIPS Technologies. */
+
+/* Performance counters implemented Per TC */
+#define MTI_CONF7_PTC		(_ULCAST_(1) << 19)
+
+/* Config7 Bits specific to BMIPS5000. */
+
+/* Performance counters implemented Per TC */
+#define BRCM_CONF7_PTC		(_ULCAST_(1) << 19)
+
 /* WatchLo* register definitions */
 #define MIPS_WATCHLO_IRW	(_ULCAST_(0x7) << 0)
 
diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index 6668f67a61c3..3308b35d6680 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -1708,6 +1708,36 @@ static const struct mips_perf_event *xlp_pmu_map_raw_event(u64 config)
 	return &raw_event;
 }
 
+#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
+/*
+ * The MIPS MT ASE specifies that performance counters may be implemented
+ * per core or per TC. If implemented per TC then all Linux CPUs have their
+ * own unique counters. If implemented per core, then VPEs in the core must
+ * treat the counters as a shared resource.
+ * Probe for the presence of per-TC counters
+ */
+static int probe_mipsmt_pertccounters(void)
+{
+	struct cpuinfo_mips *c = &current_cpu_data;
+
+	/* Non-MT cores by definition cannot implement per-TC counters */
+	if (!cpu_has_mipsmt)
+		return 0;
+
+	switch (c->processor_id & PRID_COMP_MASK) {
+	case PRID_COMP_MIPS:
+		/* MTI implementations use CONFIG7.PTC to signify presence */
+		return read_c0_config7() & MTI_CONF7_PTC;
+	case PRID_COMP_BROADCOM:
+		/* BMIPS5000 uses CONFIG7.PTC to signify presence */
+		return read_c0_config7() & BRCM_CONF7_PTC;
+	default:
+		break;
+	}
+	return 0;
+}
+#endif /* CONFIG_MIPS_PERF_SHARED_TC_COUNTERS */
+
 static int __init
 init_hw_perf_events(void)
 {
@@ -1723,7 +1753,7 @@ init_hw_perf_events(void)
 	}
 
 #ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
-	cpu_has_mipsmt_pertccounters = read_c0_config7() & (1<<19);
+	cpu_has_mipsmt_pertccounters = probe_mipsmt_pertccounters();
 	if (!cpu_has_mipsmt_pertccounters)
 		counters = counters_total_to_per_cpu(counters);
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 1/5] MIPS: perf: More robustly probe for the presence of per-tc counters
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Maciej W. Rozycki,
	Peter Zijlstra, linux-kernel, Paul Burton, Ingo Molnar,
	Jiri Olsa, Alexander Shishkin, Arnaldo Carvalho de Melo

Processors implementing the MIPS MT ASE may have performance counters
implemented per core or per TC. Processors implemented by MIPS and the
BMIPS5000 signify presence per TC through a bit in the implementation
specific Config7 register. Currently the code which probes for their
presence blindly reads a magic number corresponding to this bit, despite
it potentially having a different meaning in the CPU implementation.

Fix this by introducing probe_mipsmt_pertccounters() to probe for their
presence. This detects the ases implemented in the CPU, and reads any
implementation specific bit flagging their presence. In the case of MIPS
and BMIPS5000 implementations, this bit is Config7.PTC. A definition of
this bit is added in mipsregs.h for both MIPS Technologies
implementations and BMIPS5000.

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

The test of Config7.PTC was previously enabled when CONFIG_BMIPS5000 was
enabled, whereas now it is based on Broadcom PRID. If this bit is not
present / defined for all Broadcom implementations, please could someone
present a correct alternative means to detect it?

---
 arch/mips/include/asm/mipsregs.h     | 10 ++++++++++
 arch/mips/kernel/perf_event_mipsxx.c | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 858752dac337..4f61e16f43c3 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -684,6 +684,16 @@
 #define MIPS_CONF7_IAR		(_ULCAST_(1) << 10)
 #define MIPS_CONF7_AR		(_ULCAST_(1) << 16)
 
+/* Config7 Bits specific to MIPS Technologies. */
+
+/* Performance counters implemented Per TC */
+#define MTI_CONF7_PTC		(_ULCAST_(1) << 19)
+
+/* Config7 Bits specific to BMIPS5000. */
+
+/* Performance counters implemented Per TC */
+#define BRCM_CONF7_PTC		(_ULCAST_(1) << 19)
+
 /* WatchLo* register definitions */
 #define MIPS_WATCHLO_IRW	(_ULCAST_(0x7) << 0)
 
diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index 6668f67a61c3..3308b35d6680 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -1708,6 +1708,36 @@ static const struct mips_perf_event *xlp_pmu_map_raw_event(u64 config)
 	return &raw_event;
 }
 
+#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
+/*
+ * The MIPS MT ASE specifies that performance counters may be implemented
+ * per core or per TC. If implemented per TC then all Linux CPUs have their
+ * own unique counters. If implemented per core, then VPEs in the core must
+ * treat the counters as a shared resource.
+ * Probe for the presence of per-TC counters
+ */
+static int probe_mipsmt_pertccounters(void)
+{
+	struct cpuinfo_mips *c = &current_cpu_data;
+
+	/* Non-MT cores by definition cannot implement per-TC counters */
+	if (!cpu_has_mipsmt)
+		return 0;
+
+	switch (c->processor_id & PRID_COMP_MASK) {
+	case PRID_COMP_MIPS:
+		/* MTI implementations use CONFIG7.PTC to signify presence */
+		return read_c0_config7() & MTI_CONF7_PTC;
+	case PRID_COMP_BROADCOM:
+		/* BMIPS5000 uses CONFIG7.PTC to signify presence */
+		return read_c0_config7() & BRCM_CONF7_PTC;
+	default:
+		break;
+	}
+	return 0;
+}
+#endif /* CONFIG_MIPS_PERF_SHARED_TC_COUNTERS */
+
 static int __init
 init_hw_perf_events(void)
 {
@@ -1723,7 +1753,7 @@ init_hw_perf_events(void)
 	}
 
 #ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
-	cpu_has_mipsmt_pertccounters = read_c0_config7() & (1<<19);
+	cpu_has_mipsmt_pertccounters = probe_mipsmt_pertccounters();
 	if (!cpu_has_mipsmt_pertccounters)
 		counters = counters_total_to_per_cpu(counters);
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/5] MIPS: perf: Use correct VPE ID when setting up VPE tracing
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Peter Zijlstra,
	linux-kernel, Ingo Molnar, Jiri Olsa, Alexander Shishkin,
	Arnaldo Carvalho de Melo

There are a couple of FIXME's in the perf code which state that
cpu_data[event->cpu].vpe_id reports 0 for both CPUs. This is no longer
the case, since the vpe_id is used extensively by SMP CPS.

VPE local counting gets around this by using smp_processor_id() instead.
As it happens this does work correctly to count events on the right VPE,
but relies on 2 assumptions:
a) Always having 2 VPEs / core.
b) The hardware only paying attention to the least significant bit of
the PERFCTL.VPEID field.
If either of these assumptions change then the incorrect VPEs events
will be counted.

Fix this by replacing smp_processor_id() with
cpu_vpe_id(&current_cpu_data), in the vpe_id() macro, and pass vpe_id()
to M_PERFCTL_VPEID() when setting up PERFCTL.VPEID. The FIXME's can also
be removed since they no longer apply.

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

 arch/mips/kernel/perf_event_mipsxx.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index 3308b35d6680..bb8c6783e9a5 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -137,12 +137,8 @@ static DEFINE_RWLOCK(pmuint_rwlock);
 #define vpe_id()	(cpu_has_mipsmt_pertccounters ? \
 			 0 : (smp_processor_id() & MIPS_CPUID_TO_COUNTER_MASK))
 #else
-/*
- * FIXME: For VSMP, vpe_id() is redefined for Perf-events, because
- * cpu_data[cpuid].vpe_id reports 0 for _both_ CPUs.
- */
 #define vpe_id()	(cpu_has_mipsmt_pertccounters ? \
-			 0 : smp_processor_id())
+			 0 : cpu_vpe_id(&current_cpu_data))
 #endif
 
 /* Copied from op_model_mipsxx.c */
@@ -1279,11 +1275,7 @@ static void check_and_calc_range(struct perf_event *event,
 			 */
 			hwc->config_base |= M_TC_EN_ALL;
 		} else {
-			/*
-			 * FIXME: cpu_data[event->cpu].vpe_id reports 0
-			 * for both CPUs.
-			 */
-			hwc->config_base |= M_PERFCTL_VPEID(event->cpu);
+			hwc->config_base |= M_PERFCTL_VPEID(vpe_id());
 			hwc->config_base |= M_TC_EN_VPE;
 		}
 	} else
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/5] MIPS: perf: Use correct VPE ID when setting up VPE tracing
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Peter Zijlstra,
	linux-kernel, Ingo Molnar, Jiri Olsa, Alexander Shishkin,
	Arnaldo Carvalho de Melo

There are a couple of FIXME's in the perf code which state that
cpu_data[event->cpu].vpe_id reports 0 for both CPUs. This is no longer
the case, since the vpe_id is used extensively by SMP CPS.

VPE local counting gets around this by using smp_processor_id() instead.
As it happens this does work correctly to count events on the right VPE,
but relies on 2 assumptions:
a) Always having 2 VPEs / core.
b) The hardware only paying attention to the least significant bit of
the PERFCTL.VPEID field.
If either of these assumptions change then the incorrect VPEs events
will be counted.

Fix this by replacing smp_processor_id() with
cpu_vpe_id(&current_cpu_data), in the vpe_id() macro, and pass vpe_id()
to M_PERFCTL_VPEID() when setting up PERFCTL.VPEID. The FIXME's can also
be removed since they no longer apply.

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

 arch/mips/kernel/perf_event_mipsxx.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index 3308b35d6680..bb8c6783e9a5 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -137,12 +137,8 @@ static DEFINE_RWLOCK(pmuint_rwlock);
 #define vpe_id()	(cpu_has_mipsmt_pertccounters ? \
 			 0 : (smp_processor_id() & MIPS_CPUID_TO_COUNTER_MASK))
 #else
-/*
- * FIXME: For VSMP, vpe_id() is redefined for Perf-events, because
- * cpu_data[cpuid].vpe_id reports 0 for _both_ CPUs.
- */
 #define vpe_id()	(cpu_has_mipsmt_pertccounters ? \
-			 0 : smp_processor_id())
+			 0 : cpu_vpe_id(&current_cpu_data))
 #endif
 
 /* Copied from op_model_mipsxx.c */
@@ -1279,11 +1275,7 @@ static void check_and_calc_range(struct perf_event *event,
 			 */
 			hwc->config_base |= M_TC_EN_ALL;
 		} else {
-			/*
-			 * FIXME: cpu_data[event->cpu].vpe_id reports 0
-			 * for both CPUs.
-			 */
-			hwc->config_base |= M_PERFCTL_VPEID(event->cpu);
+			hwc->config_base |= M_PERFCTL_VPEID(vpe_id());
 			hwc->config_base |= M_TC_EN_VPE;
 		}
 	} else
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/5] MIPS: perf: Fix perf with MT counting other threads
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Peter Zijlstra,
	linux-kernel, Ingo Molnar, Jiri Olsa, Alexander Shishkin,
	Arnaldo Carvalho de Melo

When perf is used in non-system mode, i.e. without specifying CPUs to
count on, check_and_calc_range falls into the case when it sets
M_TC_EN_ALL in the counter config_base. This has the impact of always
counting for all of the threads in a core, even when the user has not
requested it. For example this can be seen with a test program which
executes 30002 instructions and 10000 branches running on one VPE and a
busy load on the other VPE in the core. Without this commit, the
expected count is not returned:

taskset 4 dd if=/dev/zero of=/dev/null count=100000 & taskset 8 perf
stat -e instructions:u,branches:u ./test_prog

 Performance counter stats for './test_prog':

            103235      instructions:u
             17015      branches:u

In order to fix this, remove check_and_calc_range entirely and perform
all of the logic in mipsxx_pmu_enable_event. Since
mipsxx_pmu_enable_event now requires the range of the event, ensure that
it is set by mipspmu_perf_event_encode in the same circumstances as
before (i.e. IS_ENABLED(CONFIG_MIPS_MT_SMP) && num_possible_cpus() > 1).

The logic of mipsxx_pmu_enable_event now becomes:
If the CPU is a BMIPS5000, then use the special vpe_id() implementation
to select which VPE to count.
If the counter has a range greater than a single VPE, i.e. it is a
core-wide counter, then ensure that the counter is set up to count
events from all TCs (though, since this is true by definition, is this
necessary? Just enabling a core-wide counter in the per-VPE case appears
experimentally to return the same counts. This is left in for now as the
logic was present before).
If the event is set up to count a particular CPU (i.e. system mode),
then the VPE ID of that CPU is used for the counter.
Otherwise, the event should be counted on the CPU scheduling this thread
(this was the critical bit missing from the previous implementation) so
the VPE ID of this CPU is used for the counter.

With this commit, the same test as before returns the counts expected:

taskset 4 dd if=/dev/zero of=/dev/null count=100000 & taskset 8 perf
stat -e instructions:u,branches:u ./test_prog

 Performance counter stats for './test_prog':

             30002      instructions:u
             10000      branches:u

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

 arch/mips/kernel/perf_event_mipsxx.c | 69 +++++++++++++++---------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index bb8c6783e9a5..782a1b6c6352 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -325,7 +325,9 @@ static int mipsxx_pmu_alloc_counter(struct cpu_hw_events *cpuc,
 
 static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
 {
+	struct perf_event *event = container_of(evt, struct perf_event, hw);
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+	unsigned int range = evt->event_base >> 24;
 
 	WARN_ON(idx < 0 || idx >= mipspmu.num_counters);
 
@@ -333,11 +335,30 @@ static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
 		(evt->config_base & M_PERFCTL_CONFIG_MASK) |
 		/* Make sure interrupt enabled. */
 		MIPS_PERFCTRL_IE;
-	if (IS_ENABLED(CONFIG_CPU_BMIPS5000))
+
+	if (IS_ENABLED(CONFIG_CPU_BMIPS5000)) {
 		/* enable the counter for the calling thread */
 		cpuc->saved_ctrl[idx] |=
 			(1 << (12 + vpe_id())) | BRCM_PERFCTRL_TC;
+	} else if (range > V) {
+		/* The counter is processor wide. Set it up to count all TCs. */
+		pr_debug("Enabling perf counter for all TCs\n");
+		cpuc->saved_ctrl[idx] |= M_TC_EN_ALL;
+	} else {
+		unsigned int cpu, ctrl;
+
+		/*
+		 * Set up the counter for a particular CPU when event->cpu is
+		 * a valid CPU number. Otherwise set up the counter for the CPU
+		 * scheduling this thread.
+		 */
+		cpu = (event->cpu >= 0) ? event->cpu : smp_processor_id();
 
+		ctrl = M_PERFCTL_VPEID(cpu_vpe_id(&cpu_data[cpu]));
+		ctrl |= M_TC_EN_VPE;
+		cpuc->saved_ctrl[idx] |= ctrl;
+		pr_debug("Enabling perf counter for CPU%d\n", cpu);
+	}
 	/*
 	 * We do not actually let the counter run. Leave it until start().
 	 */
@@ -650,14 +671,13 @@ static unsigned int mipspmu_perf_event_encode(const struct mips_perf_event *pev)
  * Top 8 bits for range, next 16 bits for cntr_mask, lowest 8 bits for
  * event_id.
  */
-#ifdef CONFIG_MIPS_MT_SMP
-	return ((unsigned int)pev->range << 24) |
-		(pev->cntr_mask & 0xffff00) |
-		(pev->event_id & 0xff);
-#else
-	return (pev->cntr_mask & 0xffff00) |
-		(pev->event_id & 0xff);
-#endif
+	if (IS_ENABLED(CONFIG_MIPS_MT_SMP) && num_possible_cpus() > 1)
+		return ((unsigned int)pev->range << 24) |
+			(pev->cntr_mask & 0xffff00) |
+			(pev->event_id & 0xff);
+	else
+		return ((pev->cntr_mask & 0xffff00) |
+			(pev->event_id & 0xff));
 }
 
 static const struct mips_perf_event *mipspmu_map_general_event(int idx)
@@ -1261,33 +1281,6 @@ static const struct mips_perf_event xlp_cache_map
 },
 };
 
-#ifdef CONFIG_MIPS_MT_SMP
-static void check_and_calc_range(struct perf_event *event,
-				 const struct mips_perf_event *pev)
-{
-	struct hw_perf_event *hwc = &event->hw;
-
-	if (event->cpu >= 0) {
-		if (pev->range > V) {
-			/*
-			 * The user selected an event that is processor
-			 * wide, while expecting it to be VPE wide.
-			 */
-			hwc->config_base |= M_TC_EN_ALL;
-		} else {
-			hwc->config_base |= M_PERFCTL_VPEID(vpe_id());
-			hwc->config_base |= M_TC_EN_VPE;
-		}
-	} else
-		hwc->config_base |= M_TC_EN_ALL;
-}
-#else
-static void check_and_calc_range(struct perf_event *event,
-				 const struct mips_perf_event *pev)
-{
-}
-#endif
-
 static int __hw_perf_event_init(struct perf_event *event)
 {
 	struct perf_event_attr *attr = &event->attr;
@@ -1323,10 +1316,6 @@ static int __hw_perf_event_init(struct perf_event *event)
 	 */
 	hwc->config_base = MIPS_PERFCTRL_IE;
 
-	/* Calculate range bits and validate it. */
-	if (num_possible_cpus() > 1)
-		check_and_calc_range(event, pev);
-
 	hwc->event_base = mipspmu_perf_event_encode(pev);
 	if (PERF_TYPE_RAW == event->attr.type)
 		mutex_unlock(&raw_event_mutex);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/5] MIPS: perf: Fix perf with MT counting other threads
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Peter Zijlstra,
	linux-kernel, Ingo Molnar, Jiri Olsa, Alexander Shishkin,
	Arnaldo Carvalho de Melo

When perf is used in non-system mode, i.e. without specifying CPUs to
count on, check_and_calc_range falls into the case when it sets
M_TC_EN_ALL in the counter config_base. This has the impact of always
counting for all of the threads in a core, even when the user has not
requested it. For example this can be seen with a test program which
executes 30002 instructions and 10000 branches running on one VPE and a
busy load on the other VPE in the core. Without this commit, the
expected count is not returned:

taskset 4 dd if=/dev/zero of=/dev/null count=100000 & taskset 8 perf
stat -e instructions:u,branches:u ./test_prog

 Performance counter stats for './test_prog':

            103235      instructions:u
             17015      branches:u

In order to fix this, remove check_and_calc_range entirely and perform
all of the logic in mipsxx_pmu_enable_event. Since
mipsxx_pmu_enable_event now requires the range of the event, ensure that
it is set by mipspmu_perf_event_encode in the same circumstances as
before (i.e. IS_ENABLED(CONFIG_MIPS_MT_SMP) && num_possible_cpus() > 1).

The logic of mipsxx_pmu_enable_event now becomes:
If the CPU is a BMIPS5000, then use the special vpe_id() implementation
to select which VPE to count.
If the counter has a range greater than a single VPE, i.e. it is a
core-wide counter, then ensure that the counter is set up to count
events from all TCs (though, since this is true by definition, is this
necessary? Just enabling a core-wide counter in the per-VPE case appears
experimentally to return the same counts. This is left in for now as the
logic was present before).
If the event is set up to count a particular CPU (i.e. system mode),
then the VPE ID of that CPU is used for the counter.
Otherwise, the event should be counted on the CPU scheduling this thread
(this was the critical bit missing from the previous implementation) so
the VPE ID of this CPU is used for the counter.

With this commit, the same test as before returns the counts expected:

taskset 4 dd if=/dev/zero of=/dev/null count=100000 & taskset 8 perf
stat -e instructions:u,branches:u ./test_prog

 Performance counter stats for './test_prog':

             30002      instructions:u
             10000      branches:u

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

 arch/mips/kernel/perf_event_mipsxx.c | 69 +++++++++++++++---------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index bb8c6783e9a5..782a1b6c6352 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -325,7 +325,9 @@ static int mipsxx_pmu_alloc_counter(struct cpu_hw_events *cpuc,
 
 static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
 {
+	struct perf_event *event = container_of(evt, struct perf_event, hw);
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+	unsigned int range = evt->event_base >> 24;
 
 	WARN_ON(idx < 0 || idx >= mipspmu.num_counters);
 
@@ -333,11 +335,30 @@ static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
 		(evt->config_base & M_PERFCTL_CONFIG_MASK) |
 		/* Make sure interrupt enabled. */
 		MIPS_PERFCTRL_IE;
-	if (IS_ENABLED(CONFIG_CPU_BMIPS5000))
+
+	if (IS_ENABLED(CONFIG_CPU_BMIPS5000)) {
 		/* enable the counter for the calling thread */
 		cpuc->saved_ctrl[idx] |=
 			(1 << (12 + vpe_id())) | BRCM_PERFCTRL_TC;
+	} else if (range > V) {
+		/* The counter is processor wide. Set it up to count all TCs. */
+		pr_debug("Enabling perf counter for all TCs\n");
+		cpuc->saved_ctrl[idx] |= M_TC_EN_ALL;
+	} else {
+		unsigned int cpu, ctrl;
+
+		/*
+		 * Set up the counter for a particular CPU when event->cpu is
+		 * a valid CPU number. Otherwise set up the counter for the CPU
+		 * scheduling this thread.
+		 */
+		cpu = (event->cpu >= 0) ? event->cpu : smp_processor_id();
 
+		ctrl = M_PERFCTL_VPEID(cpu_vpe_id(&cpu_data[cpu]));
+		ctrl |= M_TC_EN_VPE;
+		cpuc->saved_ctrl[idx] |= ctrl;
+		pr_debug("Enabling perf counter for CPU%d\n", cpu);
+	}
 	/*
 	 * We do not actually let the counter run. Leave it until start().
 	 */
@@ -650,14 +671,13 @@ static unsigned int mipspmu_perf_event_encode(const struct mips_perf_event *pev)
  * Top 8 bits for range, next 16 bits for cntr_mask, lowest 8 bits for
  * event_id.
  */
-#ifdef CONFIG_MIPS_MT_SMP
-	return ((unsigned int)pev->range << 24) |
-		(pev->cntr_mask & 0xffff00) |
-		(pev->event_id & 0xff);
-#else
-	return (pev->cntr_mask & 0xffff00) |
-		(pev->event_id & 0xff);
-#endif
+	if (IS_ENABLED(CONFIG_MIPS_MT_SMP) && num_possible_cpus() > 1)
+		return ((unsigned int)pev->range << 24) |
+			(pev->cntr_mask & 0xffff00) |
+			(pev->event_id & 0xff);
+	else
+		return ((pev->cntr_mask & 0xffff00) |
+			(pev->event_id & 0xff));
 }
 
 static const struct mips_perf_event *mipspmu_map_general_event(int idx)
@@ -1261,33 +1281,6 @@ static const struct mips_perf_event xlp_cache_map
 },
 };
 
-#ifdef CONFIG_MIPS_MT_SMP
-static void check_and_calc_range(struct perf_event *event,
-				 const struct mips_perf_event *pev)
-{
-	struct hw_perf_event *hwc = &event->hw;
-
-	if (event->cpu >= 0) {
-		if (pev->range > V) {
-			/*
-			 * The user selected an event that is processor
-			 * wide, while expecting it to be VPE wide.
-			 */
-			hwc->config_base |= M_TC_EN_ALL;
-		} else {
-			hwc->config_base |= M_PERFCTL_VPEID(vpe_id());
-			hwc->config_base |= M_TC_EN_VPE;
-		}
-	} else
-		hwc->config_base |= M_TC_EN_ALL;
-}
-#else
-static void check_and_calc_range(struct perf_event *event,
-				 const struct mips_perf_event *pev)
-{
-}
-#endif
-
 static int __hw_perf_event_init(struct perf_event *event)
 {
 	struct perf_event_attr *attr = &event->attr;
@@ -1323,10 +1316,6 @@ static int __hw_perf_event_init(struct perf_event *event)
 	 */
 	hwc->config_base = MIPS_PERFCTRL_IE;
 
-	/* Calculate range bits and validate it. */
-	if (num_possible_cpus() > 1)
-		check_and_calc_range(event, pev);
-
 	hwc->event_base = mipspmu_perf_event_encode(pev);
 	if (PERF_TYPE_RAW == event->attr.type)
 		mutex_unlock(&raw_event_mutex);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/5] MIPS: perf: Allocate per-core counters on demand
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Peter Zijlstra,
	linux-kernel, Ingo Molnar, Jiri Olsa, Alexander Shishkin,
	Arnaldo Carvalho de Melo

Previously when performance counters are per-core, rather than
per-thread, the number available were divided by 2 on detection, and the
counters used by each thread in a core were "swizzled" to ensure
separation. However, this solution is suboptimal since it relies on a
couple of assumptions:
a) Always having 2 VPEs / core (number of counters was divided by 2)
b) Always having a number of counters implemented in the core that is
   divisible by 2. For instance if an SoC implementation had a single
   counter and 2 VPEs per core, then this logic would fail and no
   performance counters would be available.
The mechanism also does not allow for one VPE in a core using more than
it's allocation of the per-core counters to count multiple events even
though other VPEs may not be using them.

Fix this situation by instead allocating (and releasing) per-core
performance counters when they are requested. This approach removes the
above assumptions and fixes the shortcomings.

In order to do this:
Add additional logic to mipsxx_pmu_alloc_counter() to detect if a
sibling is using a per-core counter, and to allocate a per-core counter
in all sibling CPUs.
Similarly, add a mipsxx_pmu_free_counter() function to release a
per-core counter in all sibling CPUs when it is finished with.
A new spinlock, core_counters_lock, is introduced to ensure exclusivity
when allocating and releasing per-core counters.
Since counters are now allocated per-core on demand, rather than being
reserved per-thread at boot, all of the "swizzling" of counters is
removed.

The upshot is that in an SoC with 2 counters / thread, counters are
reported as:
Performance counters: mips/interAptiv PMU enabled, 2 32-bit counters
available to each CPU, irq 18

Running an instance of a test program on each of 2 threads in a
core, both threads can use their 2 counters to count 2 events:

taskset 4 perf stat -e instructions:u,branches:u ./test_prog & taskset 8
perf stat -e instructions:u,branches:u ./test_prog

 Performance counter stats for './test_prog':

             30002      instructions:u
             10000      branches:u

       0.005164264 seconds time elapsed
 Performance counter stats for './test_prog':

             30002      instructions:u
             10000      branches:u

       0.006139975 seconds time elapsed

In an SoC with 2 counters / core (which can be forced by setting
cpu_has_mipsmt_pertccounters = 0), counters are reported as:
Performance counters: mips/interAptiv PMU enabled, 2 32-bit counters
available to each core, irq 18

Running an instance of a test program on each of 2 threads in a
core, now only one thread manages to secure the performance counters to
count 2 events. The other thread does not get any counters.

taskset 4 perf stat -e instructions:u,branches:u ./test_prog & taskset 8
perf stat -e instructions:u,branches:u ./test_prog

 Performance counter stats for './test_prog':

     <not counted>       instructions:u
     <not counted>       branches:u

       0.005179533 seconds time elapsed

 Performance counter stats for './test_prog':

             30002      instructions:u
             10000      branches:u

       0.005179467 seconds time elapsed

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

 arch/mips/kernel/perf_event_mipsxx.c | 130 ++++++++++++++++++++++++-----------
 1 file changed, 88 insertions(+), 42 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index 782a1b6c6352..bedb0d5ff3f2 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -131,6 +131,8 @@ static struct mips_pmu mipspmu;
 #ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
 static int cpu_has_mipsmt_pertccounters;
 
+static DEFINE_SPINLOCK(core_counters_lock);
+
 static DEFINE_RWLOCK(pmuint_rwlock);
 
 #if defined(CONFIG_CPU_BMIPS5000)
@@ -141,20 +143,6 @@ static DEFINE_RWLOCK(pmuint_rwlock);
 			 0 : cpu_vpe_id(&current_cpu_data))
 #endif
 
-/* Copied from op_model_mipsxx.c */
-static unsigned int vpe_shift(void)
-{
-	if (num_possible_cpus() > 1)
-		return 1;
-
-	return 0;
-}
-
-static unsigned int counters_total_to_per_cpu(unsigned int counters)
-{
-	return counters >> vpe_shift();
-}
-
 #else /* !CONFIG_MIPS_PERF_SHARED_TC_COUNTERS */
 #define vpe_id()	0
 
@@ -165,17 +153,8 @@ static void pause_local_counters(void);
 static irqreturn_t mipsxx_pmu_handle_irq(int, void *);
 static int mipsxx_pmu_handle_shared_irq(void);
 
-static unsigned int mipsxx_pmu_swizzle_perf_idx(unsigned int idx)
-{
-	if (vpe_id() == 1)
-		idx = (idx + 2) & 3;
-	return idx;
-}
-
 static u64 mipsxx_pmu_read_counter(unsigned int idx)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		/*
@@ -197,8 +176,6 @@ static u64 mipsxx_pmu_read_counter(unsigned int idx)
 
 static u64 mipsxx_pmu_read_counter_64(unsigned int idx)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		return read_c0_perfcntr0_64();
@@ -216,8 +193,6 @@ static u64 mipsxx_pmu_read_counter_64(unsigned int idx)
 
 static void mipsxx_pmu_write_counter(unsigned int idx, u64 val)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		write_c0_perfcntr0(val);
@@ -236,8 +211,6 @@ static void mipsxx_pmu_write_counter(unsigned int idx, u64 val)
 
 static void mipsxx_pmu_write_counter_64(unsigned int idx, u64 val)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		write_c0_perfcntr0_64(val);
@@ -256,8 +229,6 @@ static void mipsxx_pmu_write_counter_64(unsigned int idx, u64 val)
 
 static unsigned int mipsxx_pmu_read_control(unsigned int idx)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		return read_c0_perfctrl0();
@@ -275,8 +246,6 @@ static unsigned int mipsxx_pmu_read_control(unsigned int idx)
 
 static void mipsxx_pmu_write_control(unsigned int idx, unsigned int val)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		write_c0_perfctrl0(val);
@@ -296,7 +265,7 @@ static void mipsxx_pmu_write_control(unsigned int idx, unsigned int val)
 static int mipsxx_pmu_alloc_counter(struct cpu_hw_events *cpuc,
 				    struct hw_perf_event *hwc)
 {
-	int i;
+	int i, cpu = smp_processor_id();
 
 	/*
 	 * We only need to care the counter mask. The range has been
@@ -315,14 +284,88 @@ static int mipsxx_pmu_alloc_counter(struct cpu_hw_events *cpuc,
 		 * they can be dynamically swapped, they both feel happy.
 		 * But here we leave this issue alone for now.
 		 */
-		if (test_bit(i, &cntr_mask) &&
-			!test_and_set_bit(i, cpuc->used_mask))
+		if (!test_bit(i, &cntr_mask))
+			continue;
+
+#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
+		/*
+		 * When counters are per-core, check for use and allocate
+		 * them in all sibling CPUs.
+		 */
+		if (!cpu_has_mipsmt_pertccounters) {
+			struct cpu_hw_events *sibling_cpuc;
+			int sibling_cpu, allocated = 0;
+			unsigned long flags;
+
+			spin_lock_irqsave(&core_counters_lock, flags);
+
+			for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
+				sibling_cpuc = per_cpu_ptr(&cpu_hw_events,
+							   sibling_cpu);
+
+				if (test_bit(i, sibling_cpuc->used_mask)) {
+					pr_debug("CPU%d already using core counter %d\n",
+						 sibling_cpu, i);
+					goto next_counter;
+				}
+			}
+
+			/* Counter i is not used by any siblings - use it */
+			allocated = 1;
+			for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
+				sibling_cpuc = per_cpu_ptr(&cpu_hw_events,
+							   sibling_cpu);
+
+				set_bit(i, sibling_cpuc->used_mask);
+				/* sibling is using the counter */
+				pr_debug("CPU%d now using core counter %d\n",
+					 sibling_cpu, i);
+			}
+next_counter:
+			spin_unlock_irqrestore(&core_counters_lock, flags);
+			if (allocated)
+				return i;
+		}
+		else
+#endif
+		if (!test_and_set_bit(i, cpuc->used_mask)) {
+			pr_debug("CPU%d now using counter %d\n", cpu, i);
 			return i;
+		}
 	}
 
 	return -EAGAIN;
 }
 
+static void mipsxx_pmu_free_counter(struct cpu_hw_events *cpuc,
+				    struct hw_perf_event *hwc)
+{
+#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
+	int sibling_cpu, cpu = smp_processor_id();
+
+	/* When counters are per-core, free them in all sibling CPUs */
+	if (!cpu_has_mipsmt_pertccounters) {
+		struct cpu_hw_events *sibling_cpuc;
+		unsigned long flags;
+
+		spin_lock_irqsave(&core_counters_lock, flags);
+
+		for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
+			sibling_cpuc = per_cpu_ptr(&cpu_hw_events, sibling_cpu);
+
+			clear_bit(hwc->idx, sibling_cpuc->used_mask);
+			pr_debug("CPU%d released core counter %d\n",
+				 sibling_cpu, hwc->idx);
+		}
+
+		spin_unlock_irqrestore(&core_counters_lock, flags);
+		return;
+	}
+#endif
+	pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
+	clear_bit(hwc->idx, cpuc->used_mask);
+}
+
 static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
 {
 	struct perf_event *event = container_of(evt, struct perf_event, hw);
@@ -510,7 +553,7 @@ static void mipspmu_del(struct perf_event *event, int flags)
 
 	mipspmu_stop(event, PERF_EF_UPDATE);
 	cpuc->events[idx] = NULL;
-	clear_bit(idx, cpuc->used_mask);
+	mipsxx_pmu_free_counter(cpuc, hwc);
 
 	perf_event_update_userpage(event);
 }
@@ -1735,8 +1778,6 @@ init_hw_perf_events(void)
 
 #ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
 	cpu_has_mipsmt_pertccounters = probe_mipsmt_pertccounters();
-	if (!cpu_has_mipsmt_pertccounters)
-		counters = counters_total_to_per_cpu(counters);
 #endif
 
 	if (get_c0_perfcount_int)
@@ -1860,9 +1901,14 @@ init_hw_perf_events(void)
 
 	on_each_cpu(reset_counters, (void *)(long)counters, 1);
 
-	pr_cont("%s PMU enabled, %d %d-bit counters available to each "
-		"CPU, irq %d%s\n", mipspmu.name, counters, counter_bits, irq,
-		irq < 0 ? " (share with timer interrupt)" : "");
+	pr_cont("%s PMU enabled, %d %d-bit counters available to each %s, irq %d%s\n",
+		mipspmu.name, counters, counter_bits,
+#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
+		cpu_has_mipsmt_pertccounters ? "CPU" : "core",
+#else
+		"CPU",
+#endif
+		irq, irq < 0 ? " (share with timer interrupt)" : "");
 
 	perf_pmu_register(&pmu, "cpu", PERF_TYPE_RAW);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/5] MIPS: perf: Allocate per-core counters on demand
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Peter Zijlstra,
	linux-kernel, Ingo Molnar, Jiri Olsa, Alexander Shishkin,
	Arnaldo Carvalho de Melo

Previously when performance counters are per-core, rather than
per-thread, the number available were divided by 2 on detection, and the
counters used by each thread in a core were "swizzled" to ensure
separation. However, this solution is suboptimal since it relies on a
couple of assumptions:
a) Always having 2 VPEs / core (number of counters was divided by 2)
b) Always having a number of counters implemented in the core that is
   divisible by 2. For instance if an SoC implementation had a single
   counter and 2 VPEs per core, then this logic would fail and no
   performance counters would be available.
The mechanism also does not allow for one VPE in a core using more than
it's allocation of the per-core counters to count multiple events even
though other VPEs may not be using them.

Fix this situation by instead allocating (and releasing) per-core
performance counters when they are requested. This approach removes the
above assumptions and fixes the shortcomings.

In order to do this:
Add additional logic to mipsxx_pmu_alloc_counter() to detect if a
sibling is using a per-core counter, and to allocate a per-core counter
in all sibling CPUs.
Similarly, add a mipsxx_pmu_free_counter() function to release a
per-core counter in all sibling CPUs when it is finished with.
A new spinlock, core_counters_lock, is introduced to ensure exclusivity
when allocating and releasing per-core counters.
Since counters are now allocated per-core on demand, rather than being
reserved per-thread at boot, all of the "swizzling" of counters is
removed.

The upshot is that in an SoC with 2 counters / thread, counters are
reported as:
Performance counters: mips/interAptiv PMU enabled, 2 32-bit counters
available to each CPU, irq 18

Running an instance of a test program on each of 2 threads in a
core, both threads can use their 2 counters to count 2 events:

taskset 4 perf stat -e instructions:u,branches:u ./test_prog & taskset 8
perf stat -e instructions:u,branches:u ./test_prog

 Performance counter stats for './test_prog':

             30002      instructions:u
             10000      branches:u

       0.005164264 seconds time elapsed
 Performance counter stats for './test_prog':

             30002      instructions:u
             10000      branches:u

       0.006139975 seconds time elapsed

In an SoC with 2 counters / core (which can be forced by setting
cpu_has_mipsmt_pertccounters = 0), counters are reported as:
Performance counters: mips/interAptiv PMU enabled, 2 32-bit counters
available to each core, irq 18

Running an instance of a test program on each of 2 threads in a
core, now only one thread manages to secure the performance counters to
count 2 events. The other thread does not get any counters.

taskset 4 perf stat -e instructions:u,branches:u ./test_prog & taskset 8
perf stat -e instructions:u,branches:u ./test_prog

 Performance counter stats for './test_prog':

     <not counted>       instructions:u
     <not counted>       branches:u

       0.005179533 seconds time elapsed

 Performance counter stats for './test_prog':

             30002      instructions:u
             10000      branches:u

       0.005179467 seconds time elapsed

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

 arch/mips/kernel/perf_event_mipsxx.c | 130 ++++++++++++++++++++++++-----------
 1 file changed, 88 insertions(+), 42 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index 782a1b6c6352..bedb0d5ff3f2 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -131,6 +131,8 @@ static struct mips_pmu mipspmu;
 #ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
 static int cpu_has_mipsmt_pertccounters;
 
+static DEFINE_SPINLOCK(core_counters_lock);
+
 static DEFINE_RWLOCK(pmuint_rwlock);
 
 #if defined(CONFIG_CPU_BMIPS5000)
@@ -141,20 +143,6 @@ static DEFINE_RWLOCK(pmuint_rwlock);
 			 0 : cpu_vpe_id(&current_cpu_data))
 #endif
 
-/* Copied from op_model_mipsxx.c */
-static unsigned int vpe_shift(void)
-{
-	if (num_possible_cpus() > 1)
-		return 1;
-
-	return 0;
-}
-
-static unsigned int counters_total_to_per_cpu(unsigned int counters)
-{
-	return counters >> vpe_shift();
-}
-
 #else /* !CONFIG_MIPS_PERF_SHARED_TC_COUNTERS */
 #define vpe_id()	0
 
@@ -165,17 +153,8 @@ static void pause_local_counters(void);
 static irqreturn_t mipsxx_pmu_handle_irq(int, void *);
 static int mipsxx_pmu_handle_shared_irq(void);
 
-static unsigned int mipsxx_pmu_swizzle_perf_idx(unsigned int idx)
-{
-	if (vpe_id() == 1)
-		idx = (idx + 2) & 3;
-	return idx;
-}
-
 static u64 mipsxx_pmu_read_counter(unsigned int idx)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		/*
@@ -197,8 +176,6 @@ static u64 mipsxx_pmu_read_counter(unsigned int idx)
 
 static u64 mipsxx_pmu_read_counter_64(unsigned int idx)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		return read_c0_perfcntr0_64();
@@ -216,8 +193,6 @@ static u64 mipsxx_pmu_read_counter_64(unsigned int idx)
 
 static void mipsxx_pmu_write_counter(unsigned int idx, u64 val)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		write_c0_perfcntr0(val);
@@ -236,8 +211,6 @@ static void mipsxx_pmu_write_counter(unsigned int idx, u64 val)
 
 static void mipsxx_pmu_write_counter_64(unsigned int idx, u64 val)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		write_c0_perfcntr0_64(val);
@@ -256,8 +229,6 @@ static void mipsxx_pmu_write_counter_64(unsigned int idx, u64 val)
 
 static unsigned int mipsxx_pmu_read_control(unsigned int idx)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		return read_c0_perfctrl0();
@@ -275,8 +246,6 @@ static unsigned int mipsxx_pmu_read_control(unsigned int idx)
 
 static void mipsxx_pmu_write_control(unsigned int idx, unsigned int val)
 {
-	idx = mipsxx_pmu_swizzle_perf_idx(idx);
-
 	switch (idx) {
 	case 0:
 		write_c0_perfctrl0(val);
@@ -296,7 +265,7 @@ static void mipsxx_pmu_write_control(unsigned int idx, unsigned int val)
 static int mipsxx_pmu_alloc_counter(struct cpu_hw_events *cpuc,
 				    struct hw_perf_event *hwc)
 {
-	int i;
+	int i, cpu = smp_processor_id();
 
 	/*
 	 * We only need to care the counter mask. The range has been
@@ -315,14 +284,88 @@ static int mipsxx_pmu_alloc_counter(struct cpu_hw_events *cpuc,
 		 * they can be dynamically swapped, they both feel happy.
 		 * But here we leave this issue alone for now.
 		 */
-		if (test_bit(i, &cntr_mask) &&
-			!test_and_set_bit(i, cpuc->used_mask))
+		if (!test_bit(i, &cntr_mask))
+			continue;
+
+#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
+		/*
+		 * When counters are per-core, check for use and allocate
+		 * them in all sibling CPUs.
+		 */
+		if (!cpu_has_mipsmt_pertccounters) {
+			struct cpu_hw_events *sibling_cpuc;
+			int sibling_cpu, allocated = 0;
+			unsigned long flags;
+
+			spin_lock_irqsave(&core_counters_lock, flags);
+
+			for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
+				sibling_cpuc = per_cpu_ptr(&cpu_hw_events,
+							   sibling_cpu);
+
+				if (test_bit(i, sibling_cpuc->used_mask)) {
+					pr_debug("CPU%d already using core counter %d\n",
+						 sibling_cpu, i);
+					goto next_counter;
+				}
+			}
+
+			/* Counter i is not used by any siblings - use it */
+			allocated = 1;
+			for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
+				sibling_cpuc = per_cpu_ptr(&cpu_hw_events,
+							   sibling_cpu);
+
+				set_bit(i, sibling_cpuc->used_mask);
+				/* sibling is using the counter */
+				pr_debug("CPU%d now using core counter %d\n",
+					 sibling_cpu, i);
+			}
+next_counter:
+			spin_unlock_irqrestore(&core_counters_lock, flags);
+			if (allocated)
+				return i;
+		}
+		else
+#endif
+		if (!test_and_set_bit(i, cpuc->used_mask)) {
+			pr_debug("CPU%d now using counter %d\n", cpu, i);
 			return i;
+		}
 	}
 
 	return -EAGAIN;
 }
 
+static void mipsxx_pmu_free_counter(struct cpu_hw_events *cpuc,
+				    struct hw_perf_event *hwc)
+{
+#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
+	int sibling_cpu, cpu = smp_processor_id();
+
+	/* When counters are per-core, free them in all sibling CPUs */
+	if (!cpu_has_mipsmt_pertccounters) {
+		struct cpu_hw_events *sibling_cpuc;
+		unsigned long flags;
+
+		spin_lock_irqsave(&core_counters_lock, flags);
+
+		for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
+			sibling_cpuc = per_cpu_ptr(&cpu_hw_events, sibling_cpu);
+
+			clear_bit(hwc->idx, sibling_cpuc->used_mask);
+			pr_debug("CPU%d released core counter %d\n",
+				 sibling_cpu, hwc->idx);
+		}
+
+		spin_unlock_irqrestore(&core_counters_lock, flags);
+		return;
+	}
+#endif
+	pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
+	clear_bit(hwc->idx, cpuc->used_mask);
+}
+
 static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
 {
 	struct perf_event *event = container_of(evt, struct perf_event, hw);
@@ -510,7 +553,7 @@ static void mipspmu_del(struct perf_event *event, int flags)
 
 	mipspmu_stop(event, PERF_EF_UPDATE);
 	cpuc->events[idx] = NULL;
-	clear_bit(idx, cpuc->used_mask);
+	mipsxx_pmu_free_counter(cpuc, hwc);
 
 	perf_event_update_userpage(event);
 }
@@ -1735,8 +1778,6 @@ init_hw_perf_events(void)
 
 #ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
 	cpu_has_mipsmt_pertccounters = probe_mipsmt_pertccounters();
-	if (!cpu_has_mipsmt_pertccounters)
-		counters = counters_total_to_per_cpu(counters);
 #endif
 
 	if (get_c0_perfcount_int)
@@ -1860,9 +1901,14 @@ init_hw_perf_events(void)
 
 	on_each_cpu(reset_counters, (void *)(long)counters, 1);
 
-	pr_cont("%s PMU enabled, %d %d-bit counters available to each "
-		"CPU, irq %d%s\n", mipspmu.name, counters, counter_bits, irq,
-		irq < 0 ? " (share with timer interrupt)" : "");
+	pr_cont("%s PMU enabled, %d %d-bit counters available to each %s, irq %d%s\n",
+		mipspmu.name, counters, counter_bits,
+#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
+		cpu_has_mipsmt_pertccounters ? "CPU" : "core",
+#else
+		"CPU",
+#endif
+		irq, irq < 0 ? " (share with timer interrupt)" : "");
 
 	perf_pmu_register(&pmu, "cpu", PERF_TYPE_RAW);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/5] MIPS: perf: Fold vpe_id() macro into it's one last usage
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Peter Zijlstra,
	linux-kernel, Ingo Molnar, Jiri Olsa, Alexander Shishkin,
	Arnaldo Carvalho de Melo

The vpe_id() macro is now used only in mipsxx_pmu_enable_event when
CONFIG_CPU_BMIPS5000 is defined. Fold the one used definition of the
macro into it's usage and remove the now unused definitions.

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

 arch/mips/kernel/perf_event_mipsxx.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index bedb0d5ff3f2..44b9090a53cc 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -134,18 +134,6 @@ static int cpu_has_mipsmt_pertccounters;
 static DEFINE_SPINLOCK(core_counters_lock);
 
 static DEFINE_RWLOCK(pmuint_rwlock);
-
-#if defined(CONFIG_CPU_BMIPS5000)
-#define vpe_id()	(cpu_has_mipsmt_pertccounters ? \
-			 0 : (smp_processor_id() & MIPS_CPUID_TO_COUNTER_MASK))
-#else
-#define vpe_id()	(cpu_has_mipsmt_pertccounters ? \
-			 0 : cpu_vpe_id(&current_cpu_data))
-#endif
-
-#else /* !CONFIG_MIPS_PERF_SHARED_TC_COUNTERS */
-#define vpe_id()	0
-
 #endif /* CONFIG_MIPS_PERF_SHARED_TC_COUNTERS */
 
 static void resume_local_counters(void);
@@ -381,8 +369,10 @@ static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
 
 	if (IS_ENABLED(CONFIG_CPU_BMIPS5000)) {
 		/* enable the counter for the calling thread */
-		cpuc->saved_ctrl[idx] |=
-			(1 << (12 + vpe_id())) | BRCM_PERFCTRL_TC;
+		unsigned int vpe_id = cpu_has_mipsmt_pertccounters ? 0 :
+			(smp_processor_id() & MIPS_CPUID_TO_COUNTER_MASK);
+
+		cpuc->saved_ctrl[idx] |= BIT(12 + vpe_id) | BRCM_PERFCTRL_TC;
 	} else if (range > V) {
 		/* The counter is processor wide. Set it up to count all TCs. */
 		pr_debug("Enabling perf counter for all TCs\n");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/5] MIPS: perf: Fold vpe_id() macro into it's one last usage
@ 2018-04-03 12:31   ` Matt Redfearn
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Redfearn @ 2018-04-03 12:31 UTC (permalink / raw)
  To: James Hogan, Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Namhyung Kim, Peter Zijlstra,
	linux-kernel, Ingo Molnar, Jiri Olsa, Alexander Shishkin,
	Arnaldo Carvalho de Melo

The vpe_id() macro is now used only in mipsxx_pmu_enable_event when
CONFIG_CPU_BMIPS5000 is defined. Fold the one used definition of the
macro into it's usage and remove the now unused definitions.

Signed-off-by: Matt Redfearn <matt.redfearn@mips.com>
---

 arch/mips/kernel/perf_event_mipsxx.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index bedb0d5ff3f2..44b9090a53cc 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -134,18 +134,6 @@ static int cpu_has_mipsmt_pertccounters;
 static DEFINE_SPINLOCK(core_counters_lock);
 
 static DEFINE_RWLOCK(pmuint_rwlock);
-
-#if defined(CONFIG_CPU_BMIPS5000)
-#define vpe_id()	(cpu_has_mipsmt_pertccounters ? \
-			 0 : (smp_processor_id() & MIPS_CPUID_TO_COUNTER_MASK))
-#else
-#define vpe_id()	(cpu_has_mipsmt_pertccounters ? \
-			 0 : cpu_vpe_id(&current_cpu_data))
-#endif
-
-#else /* !CONFIG_MIPS_PERF_SHARED_TC_COUNTERS */
-#define vpe_id()	0
-
 #endif /* CONFIG_MIPS_PERF_SHARED_TC_COUNTERS */
 
 static void resume_local_counters(void);
@@ -381,8 +369,10 @@ static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
 
 	if (IS_ENABLED(CONFIG_CPU_BMIPS5000)) {
 		/* enable the counter for the calling thread */
-		cpuc->saved_ctrl[idx] |=
-			(1 << (12 + vpe_id())) | BRCM_PERFCTRL_TC;
+		unsigned int vpe_id = cpu_has_mipsmt_pertccounters ? 0 :
+			(smp_processor_id() & MIPS_CPUID_TO_COUNTER_MASK);
+
+		cpuc->saved_ctrl[idx] |= BIT(12 + vpe_id) | BRCM_PERFCTRL_TC;
 	} else if (range > V) {
 		/* The counter is processor wide. Set it up to count all TCs. */
 		pr_debug("Enabling perf counter for all TCs\n");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] MIPS: perf: Fix perf with MT counting other threads
@ 2018-04-03 19:49     ` kbuild test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2018-04-03 19:49 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: kbuild-all, James Hogan, Ralf Baechle, linux-mips, Matt Redfearn,
	Namhyung Kim, Peter Zijlstra, linux-kernel, Ingo Molnar,
	Jiri Olsa, Alexander Shishkin, Arnaldo Carvalho de Melo

[-- Attachment #1: Type: text/plain, Size: 3092 bytes --]

Hi Matt,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.16 next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Redfearn/MIPS-perf-MT-fixes-and-improvements/20180404-011026
config: mips-gpr_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_enable_event':
>> arch/mips/kernel/perf_event_mipsxx.c:343:22: error: expected expression before ')' token
     } else if (range > V) {
                         ^
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipspmu_perf_event_encode':
>> arch/mips/kernel/perf_event_mipsxx.c:675:28: error: 'const struct mips_perf_event' has no member named 'range'
      return ((unsigned int)pev->range << 24) |
                               ^~

vim +343 arch/mips/kernel/perf_event_mipsxx.c

   325	
   326	static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
   327	{
   328		struct perf_event *event = container_of(evt, struct perf_event, hw);
   329		struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
   330		unsigned int range = evt->event_base >> 24;
   331	
   332		WARN_ON(idx < 0 || idx >= mipspmu.num_counters);
   333	
   334		cpuc->saved_ctrl[idx] = M_PERFCTL_EVENT(evt->event_base & 0xff) |
   335			(evt->config_base & M_PERFCTL_CONFIG_MASK) |
   336			/* Make sure interrupt enabled. */
   337			MIPS_PERFCTRL_IE;
   338	
   339		if (IS_ENABLED(CONFIG_CPU_BMIPS5000)) {
   340			/* enable the counter for the calling thread */
   341			cpuc->saved_ctrl[idx] |=
   342				(1 << (12 + vpe_id())) | BRCM_PERFCTRL_TC;
 > 343		} else if (range > V) {
   344			/* The counter is processor wide. Set it up to count all TCs. */
   345			pr_debug("Enabling perf counter for all TCs\n");
   346			cpuc->saved_ctrl[idx] |= M_TC_EN_ALL;
   347		} else {
   348			unsigned int cpu, ctrl;
   349	
   350			/*
   351			 * Set up the counter for a particular CPU when event->cpu is
   352			 * a valid CPU number. Otherwise set up the counter for the CPU
   353			 * scheduling this thread.
   354			 */
   355			cpu = (event->cpu >= 0) ? event->cpu : smp_processor_id();
   356	
   357			ctrl = M_PERFCTL_VPEID(cpu_vpe_id(&cpu_data[cpu]));
   358			ctrl |= M_TC_EN_VPE;
   359			cpuc->saved_ctrl[idx] |= ctrl;
   360			pr_debug("Enabling perf counter for CPU%d\n", cpu);
   361		}
   362		/*
   363		 * We do not actually let the counter run. Leave it until start().
   364		 */
   365	}
   366	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19550 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] MIPS: perf: Fix perf with MT counting other threads
@ 2018-04-03 19:49     ` kbuild test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2018-04-03 19:49 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: kbuild-all, James Hogan, Ralf Baechle, linux-mips, Namhyung Kim,
	Peter Zijlstra, linux-kernel, Ingo Molnar, Jiri Olsa,
	Alexander Shishkin, Arnaldo Carvalho de Melo

[-- Attachment #1: Type: text/plain, Size: 3092 bytes --]

Hi Matt,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.16 next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Redfearn/MIPS-perf-MT-fixes-and-improvements/20180404-011026
config: mips-gpr_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_enable_event':
>> arch/mips/kernel/perf_event_mipsxx.c:343:22: error: expected expression before ')' token
     } else if (range > V) {
                         ^
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipspmu_perf_event_encode':
>> arch/mips/kernel/perf_event_mipsxx.c:675:28: error: 'const struct mips_perf_event' has no member named 'range'
      return ((unsigned int)pev->range << 24) |
                               ^~

vim +343 arch/mips/kernel/perf_event_mipsxx.c

   325	
   326	static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
   327	{
   328		struct perf_event *event = container_of(evt, struct perf_event, hw);
   329		struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
   330		unsigned int range = evt->event_base >> 24;
   331	
   332		WARN_ON(idx < 0 || idx >= mipspmu.num_counters);
   333	
   334		cpuc->saved_ctrl[idx] = M_PERFCTL_EVENT(evt->event_base & 0xff) |
   335			(evt->config_base & M_PERFCTL_CONFIG_MASK) |
   336			/* Make sure interrupt enabled. */
   337			MIPS_PERFCTRL_IE;
   338	
   339		if (IS_ENABLED(CONFIG_CPU_BMIPS5000)) {
   340			/* enable the counter for the calling thread */
   341			cpuc->saved_ctrl[idx] |=
   342				(1 << (12 + vpe_id())) | BRCM_PERFCTRL_TC;
 > 343		} else if (range > V) {
   344			/* The counter is processor wide. Set it up to count all TCs. */
   345			pr_debug("Enabling perf counter for all TCs\n");
   346			cpuc->saved_ctrl[idx] |= M_TC_EN_ALL;
   347		} else {
   348			unsigned int cpu, ctrl;
   349	
   350			/*
   351			 * Set up the counter for a particular CPU when event->cpu is
   352			 * a valid CPU number. Otherwise set up the counter for the CPU
   353			 * scheduling this thread.
   354			 */
   355			cpu = (event->cpu >= 0) ? event->cpu : smp_processor_id();
   356	
   357			ctrl = M_PERFCTL_VPEID(cpu_vpe_id(&cpu_data[cpu]));
   358			ctrl |= M_TC_EN_VPE;
   359			cpuc->saved_ctrl[idx] |= ctrl;
   360			pr_debug("Enabling perf counter for CPU%d\n", cpu);
   361		}
   362		/*
   363		 * We do not actually let the counter run. Leave it until start().
   364		 */
   365	}
   366	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19550 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/5] MIPS: perf: Allocate per-core counters on demand
@ 2018-04-03 20:34     ` kbuild test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2018-04-03 20:34 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: kbuild-all, James Hogan, Ralf Baechle, linux-mips, Matt Redfearn,
	Namhyung Kim, Peter Zijlstra, linux-kernel, Ingo Molnar,
	Jiri Olsa, Alexander Shishkin, Arnaldo Carvalho de Melo

[-- Attachment #1: Type: text/plain, Size: 3823 bytes --]

Hi Matt,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.16 next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Redfearn/MIPS-perf-MT-fixes-and-improvements/20180404-011026
config: mips-gpr_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from include/linux/cpumask.h:10,
                    from arch/mips/kernel/perf_event_mipsxx.c:18:
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_free_counter':
>> arch/mips/kernel/perf_event_mipsxx.c:365:42: error: 'cpu' undeclared (first use in this function)
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
                                             ^
   include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
                    ^~~~~~~~~~~
>> arch/mips/kernel/perf_event_mipsxx.c:365:2: note: in expansion of macro 'pr_debug'
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
     ^~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c:365:42: note: each undeclared identifier is reported only once for each function it appears in
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
                                             ^
   include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
                    ^~~~~~~~~~~
>> arch/mips/kernel/perf_event_mipsxx.c:365:2: note: in expansion of macro 'pr_debug'
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
     ^~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_enable_event':
   arch/mips/kernel/perf_event_mipsxx.c:386:22: error: expected expression before ')' token
     } else if (range > V) {
                         ^
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipspmu_perf_event_encode':
   arch/mips/kernel/perf_event_mipsxx.c:718:28: error: 'const struct mips_perf_event' has no member named 'range'
      return ((unsigned int)pev->range << 24) |
                               ^~

vim +/cpu +365 arch/mips/kernel/perf_event_mipsxx.c

   339	
   340	static void mipsxx_pmu_free_counter(struct cpu_hw_events *cpuc,
   341					    struct hw_perf_event *hwc)
   342	{
   343	#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
   344		int sibling_cpu, cpu = smp_processor_id();
   345	
   346		/* When counters are per-core, free them in all sibling CPUs */
   347		if (!cpu_has_mipsmt_pertccounters) {
   348			struct cpu_hw_events *sibling_cpuc;
   349			unsigned long flags;
   350	
   351			spin_lock_irqsave(&core_counters_lock, flags);
   352	
   353			for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
   354				sibling_cpuc = per_cpu_ptr(&cpu_hw_events, sibling_cpu);
   355	
   356				clear_bit(hwc->idx, sibling_cpuc->used_mask);
   357				pr_debug("CPU%d released core counter %d\n",
   358					 sibling_cpu, hwc->idx);
   359			}
   360	
   361			spin_unlock_irqrestore(&core_counters_lock, flags);
   362			return;
   363		}
   364	#endif
 > 365		pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
   366		clear_bit(hwc->idx, cpuc->used_mask);
   367	}
   368	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19550 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/5] MIPS: perf: Allocate per-core counters on demand
@ 2018-04-03 20:34     ` kbuild test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2018-04-03 20:34 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: kbuild-all, James Hogan, Ralf Baechle, linux-mips, Namhyung Kim,
	Peter Zijlstra, linux-kernel, Ingo Molnar, Jiri Olsa,
	Alexander Shishkin, Arnaldo Carvalho de Melo

[-- Attachment #1: Type: text/plain, Size: 3823 bytes --]

Hi Matt,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.16 next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Redfearn/MIPS-perf-MT-fixes-and-improvements/20180404-011026
config: mips-gpr_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from include/linux/cpumask.h:10,
                    from arch/mips/kernel/perf_event_mipsxx.c:18:
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_free_counter':
>> arch/mips/kernel/perf_event_mipsxx.c:365:42: error: 'cpu' undeclared (first use in this function)
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
                                             ^
   include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
                    ^~~~~~~~~~~
>> arch/mips/kernel/perf_event_mipsxx.c:365:2: note: in expansion of macro 'pr_debug'
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
     ^~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c:365:42: note: each undeclared identifier is reported only once for each function it appears in
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
                                             ^
   include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
                    ^~~~~~~~~~~
>> arch/mips/kernel/perf_event_mipsxx.c:365:2: note: in expansion of macro 'pr_debug'
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
     ^~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_enable_event':
   arch/mips/kernel/perf_event_mipsxx.c:386:22: error: expected expression before ')' token
     } else if (range > V) {
                         ^
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipspmu_perf_event_encode':
   arch/mips/kernel/perf_event_mipsxx.c:718:28: error: 'const struct mips_perf_event' has no member named 'range'
      return ((unsigned int)pev->range << 24) |
                               ^~

vim +/cpu +365 arch/mips/kernel/perf_event_mipsxx.c

   339	
   340	static void mipsxx_pmu_free_counter(struct cpu_hw_events *cpuc,
   341					    struct hw_perf_event *hwc)
   342	{
   343	#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
   344		int sibling_cpu, cpu = smp_processor_id();
   345	
   346		/* When counters are per-core, free them in all sibling CPUs */
   347		if (!cpu_has_mipsmt_pertccounters) {
   348			struct cpu_hw_events *sibling_cpuc;
   349			unsigned long flags;
   350	
   351			spin_lock_irqsave(&core_counters_lock, flags);
   352	
   353			for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
   354				sibling_cpuc = per_cpu_ptr(&cpu_hw_events, sibling_cpu);
   355	
   356				clear_bit(hwc->idx, sibling_cpuc->used_mask);
   357				pr_debug("CPU%d released core counter %d\n",
   358					 sibling_cpu, hwc->idx);
   359			}
   360	
   361			spin_unlock_irqrestore(&core_counters_lock, flags);
   362			return;
   363		}
   364	#endif
 > 365		pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
   366		clear_bit(hwc->idx, cpuc->used_mask);
   367	}
   368	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19550 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/5] MIPS: perf: Fold vpe_id() macro into it's one last usage
@ 2018-04-03 20:57     ` kbuild test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2018-04-03 20:57 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: kbuild-all, James Hogan, Ralf Baechle, linux-mips, Matt Redfearn,
	Namhyung Kim, Peter Zijlstra, linux-kernel, Ingo Molnar,
	Jiri Olsa, Alexander Shishkin, Arnaldo Carvalho de Melo

[-- Attachment #1: Type: text/plain, Size: 5884 bytes --]

Hi Matt,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.16 next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Redfearn/MIPS-perf-MT-fixes-and-improvements/20180404-011026
config: mips-gpr_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from include/linux/cpumask.h:10,
                    from arch/mips/kernel/perf_event_mipsxx.c:18:
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_free_counter':
   arch/mips/kernel/perf_event_mipsxx.c:353:42: error: 'cpu' undeclared (first use in this function)
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
                                             ^
   include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
                    ^~~~~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c:353:2: note: in expansion of macro 'pr_debug'
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
     ^~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c:353:42: note: each undeclared identifier is reported only once for each function it appears in
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
                                             ^
   include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
                    ^~~~~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c:353:2: note: in expansion of macro 'pr_debug'
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
     ^~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_enable_event':
>> arch/mips/kernel/perf_event_mipsxx.c:372:25: error: 'cpu_has_mipsmt_pertccounters' undeclared (first use in this function); did you mean 'can_use_mips_counter'?
      unsigned int vpe_id = cpu_has_mipsmt_pertccounters ? 0 :
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
                            can_use_mips_counter
   arch/mips/kernel/perf_event_mipsxx.c:376:22: error: expected expression before ')' token
     } else if (range > V) {
                         ^
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipspmu_perf_event_encode':
   arch/mips/kernel/perf_event_mipsxx.c:708:28: error: 'const struct mips_perf_event' has no member named 'range'
      return ((unsigned int)pev->range << 24) |
                               ^~

vim +372 arch/mips/kernel/perf_event_mipsxx.c

   327	
   328	static void mipsxx_pmu_free_counter(struct cpu_hw_events *cpuc,
   329					    struct hw_perf_event *hwc)
   330	{
   331	#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
   332		int sibling_cpu, cpu = smp_processor_id();
   333	
   334		/* When counters are per-core, free them in all sibling CPUs */
   335		if (!cpu_has_mipsmt_pertccounters) {
   336			struct cpu_hw_events *sibling_cpuc;
   337			unsigned long flags;
   338	
   339			spin_lock_irqsave(&core_counters_lock, flags);
   340	
   341			for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
   342				sibling_cpuc = per_cpu_ptr(&cpu_hw_events, sibling_cpu);
   343	
   344				clear_bit(hwc->idx, sibling_cpuc->used_mask);
   345				pr_debug("CPU%d released core counter %d\n",
   346					 sibling_cpu, hwc->idx);
   347			}
   348	
   349			spin_unlock_irqrestore(&core_counters_lock, flags);
   350			return;
   351		}
   352	#endif
 > 353		pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
   354		clear_bit(hwc->idx, cpuc->used_mask);
   355	}
   356	
   357	static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
   358	{
   359		struct perf_event *event = container_of(evt, struct perf_event, hw);
   360		struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
   361		unsigned int range = evt->event_base >> 24;
   362	
   363		WARN_ON(idx < 0 || idx >= mipspmu.num_counters);
   364	
   365		cpuc->saved_ctrl[idx] = M_PERFCTL_EVENT(evt->event_base & 0xff) |
   366			(evt->config_base & M_PERFCTL_CONFIG_MASK) |
   367			/* Make sure interrupt enabled. */
   368			MIPS_PERFCTRL_IE;
   369	
   370		if (IS_ENABLED(CONFIG_CPU_BMIPS5000)) {
   371			/* enable the counter for the calling thread */
 > 372			unsigned int vpe_id = cpu_has_mipsmt_pertccounters ? 0 :
   373				(smp_processor_id() & MIPS_CPUID_TO_COUNTER_MASK);
   374	
   375			cpuc->saved_ctrl[idx] |= BIT(12 + vpe_id) | BRCM_PERFCTRL_TC;
   376		} else if (range > V) {
   377			/* The counter is processor wide. Set it up to count all TCs. */
   378			pr_debug("Enabling perf counter for all TCs\n");
   379			cpuc->saved_ctrl[idx] |= M_TC_EN_ALL;
   380		} else {
   381			unsigned int cpu, ctrl;
   382	
   383			/*
   384			 * Set up the counter for a particular CPU when event->cpu is
   385			 * a valid CPU number. Otherwise set up the counter for the CPU
   386			 * scheduling this thread.
   387			 */
   388			cpu = (event->cpu >= 0) ? event->cpu : smp_processor_id();
   389	
   390			ctrl = M_PERFCTL_VPEID(cpu_vpe_id(&cpu_data[cpu]));
   391			ctrl |= M_TC_EN_VPE;
   392			cpuc->saved_ctrl[idx] |= ctrl;
   393			pr_debug("Enabling perf counter for CPU%d\n", cpu);
   394		}
   395		/*
   396		 * We do not actually let the counter run. Leave it until start().
   397		 */
   398	}
   399	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19550 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/5] MIPS: perf: Fold vpe_id() macro into it's one last usage
@ 2018-04-03 20:57     ` kbuild test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2018-04-03 20:57 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: kbuild-all, James Hogan, Ralf Baechle, linux-mips, Namhyung Kim,
	Peter Zijlstra, linux-kernel, Ingo Molnar, Jiri Olsa,
	Alexander Shishkin, Arnaldo Carvalho de Melo

[-- Attachment #1: Type: text/plain, Size: 5884 bytes --]

Hi Matt,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.16 next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Redfearn/MIPS-perf-MT-fixes-and-improvements/20180404-011026
config: mips-gpr_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from include/linux/cpumask.h:10,
                    from arch/mips/kernel/perf_event_mipsxx.c:18:
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_free_counter':
   arch/mips/kernel/perf_event_mipsxx.c:353:42: error: 'cpu' undeclared (first use in this function)
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
                                             ^
   include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
                    ^~~~~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c:353:2: note: in expansion of macro 'pr_debug'
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
     ^~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c:353:42: note: each undeclared identifier is reported only once for each function it appears in
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
                                             ^
   include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
                    ^~~~~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c:353:2: note: in expansion of macro 'pr_debug'
     pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
     ^~~~~~~~
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipsxx_pmu_enable_event':
>> arch/mips/kernel/perf_event_mipsxx.c:372:25: error: 'cpu_has_mipsmt_pertccounters' undeclared (first use in this function); did you mean 'can_use_mips_counter'?
      unsigned int vpe_id = cpu_has_mipsmt_pertccounters ? 0 :
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
                            can_use_mips_counter
   arch/mips/kernel/perf_event_mipsxx.c:376:22: error: expected expression before ')' token
     } else if (range > V) {
                         ^
   arch/mips/kernel/perf_event_mipsxx.c: In function 'mipspmu_perf_event_encode':
   arch/mips/kernel/perf_event_mipsxx.c:708:28: error: 'const struct mips_perf_event' has no member named 'range'
      return ((unsigned int)pev->range << 24) |
                               ^~

vim +372 arch/mips/kernel/perf_event_mipsxx.c

   327	
   328	static void mipsxx_pmu_free_counter(struct cpu_hw_events *cpuc,
   329					    struct hw_perf_event *hwc)
   330	{
   331	#ifdef CONFIG_MIPS_PERF_SHARED_TC_COUNTERS
   332		int sibling_cpu, cpu = smp_processor_id();
   333	
   334		/* When counters are per-core, free them in all sibling CPUs */
   335		if (!cpu_has_mipsmt_pertccounters) {
   336			struct cpu_hw_events *sibling_cpuc;
   337			unsigned long flags;
   338	
   339			spin_lock_irqsave(&core_counters_lock, flags);
   340	
   341			for_each_cpu(sibling_cpu, &cpu_sibling_map[cpu]) {
   342				sibling_cpuc = per_cpu_ptr(&cpu_hw_events, sibling_cpu);
   343	
   344				clear_bit(hwc->idx, sibling_cpuc->used_mask);
   345				pr_debug("CPU%d released core counter %d\n",
   346					 sibling_cpu, hwc->idx);
   347			}
   348	
   349			spin_unlock_irqrestore(&core_counters_lock, flags);
   350			return;
   351		}
   352	#endif
 > 353		pr_debug("CPU%d released counter %d\n", cpu, hwc->idx);
   354		clear_bit(hwc->idx, cpuc->used_mask);
   355	}
   356	
   357	static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx)
   358	{
   359		struct perf_event *event = container_of(evt, struct perf_event, hw);
   360		struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
   361		unsigned int range = evt->event_base >> 24;
   362	
   363		WARN_ON(idx < 0 || idx >= mipspmu.num_counters);
   364	
   365		cpuc->saved_ctrl[idx] = M_PERFCTL_EVENT(evt->event_base & 0xff) |
   366			(evt->config_base & M_PERFCTL_CONFIG_MASK) |
   367			/* Make sure interrupt enabled. */
   368			MIPS_PERFCTRL_IE;
   369	
   370		if (IS_ENABLED(CONFIG_CPU_BMIPS5000)) {
   371			/* enable the counter for the calling thread */
 > 372			unsigned int vpe_id = cpu_has_mipsmt_pertccounters ? 0 :
   373				(smp_processor_id() & MIPS_CPUID_TO_COUNTER_MASK);
   374	
   375			cpuc->saved_ctrl[idx] |= BIT(12 + vpe_id) | BRCM_PERFCTRL_TC;
   376		} else if (range > V) {
   377			/* The counter is processor wide. Set it up to count all TCs. */
   378			pr_debug("Enabling perf counter for all TCs\n");
   379			cpuc->saved_ctrl[idx] |= M_TC_EN_ALL;
   380		} else {
   381			unsigned int cpu, ctrl;
   382	
   383			/*
   384			 * Set up the counter for a particular CPU when event->cpu is
   385			 * a valid CPU number. Otherwise set up the counter for the CPU
   386			 * scheduling this thread.
   387			 */
   388			cpu = (event->cpu >= 0) ? event->cpu : smp_processor_id();
   389	
   390			ctrl = M_PERFCTL_VPEID(cpu_vpe_id(&cpu_data[cpu]));
   391			ctrl |= M_TC_EN_VPE;
   392			cpuc->saved_ctrl[idx] |= ctrl;
   393			pr_debug("Enabling perf counter for CPU%d\n", cpu);
   394		}
   395		/*
   396		 * We do not actually let the counter run. Leave it until start().
   397		 */
   398	}
   399	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19550 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] MIPS: perf: More robustly probe for the presence of per-tc counters
  2018-04-03 12:31   ` Matt Redfearn
  (?)
@ 2018-04-05 16:43   ` Sasha Levin
  -1 siblings, 0 replies; 20+ messages in thread
From: Sasha Levin @ 2018-04-05 16:43 UTC (permalink / raw)
  To: Sasha Levin, Matt Redfearn, James Hogan, Ralf Baechle; +Cc: linux-mips, stable

Hi.

[This is an automated email]

This commit has been processed by the -stable helper bot and determined
to be a high probability candidate for -stable trees. (score: 7.7428)

The bot has tested the following trees: v4.15.15, v4.14.32, v4.9.92, v4.4.126, 

v4.15.15: Build OK!
v4.14.32: Build OK!
v4.9.92: Build OK!
v4.4.126: Failed to apply! Possible dependencies:
    dddf796db4e3 ("MIPS: perf: More robustly probe for the presence of per-tc counters")


Please let us know if you'd like to have this patch included in a stable tree.

--
Thanks.
Sasha

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] MIPS: perf: Fix perf with MT counting other threads
  2018-04-03 12:31   ` Matt Redfearn
  (?)
  (?)
@ 2018-04-05 16:43   ` Sasha Levin
  -1 siblings, 0 replies; 20+ messages in thread
From: Sasha Levin @ 2018-04-05 16:43 UTC (permalink / raw)
  To: Sasha Levin, Matt Redfearn, James Hogan, Ralf Baechle; +Cc: linux-mips, stable

Hi.

[This is an automated email]

This commit has been processed by the -stable helper bot and determined
to be a high probability candidate for -stable trees. (score: 16.4885)

The bot has tested the following trees: v4.15.15, v4.14.32, v4.9.92, v4.4.126, 

v4.15.15: Failed to apply! Possible dependencies:
    e2a7b77d89f6 ("MIPS: perf: Fix perf with MT counting other threads")

v4.14.32: Failed to apply! Possible dependencies:
    e2a7b77d89f6 ("MIPS: perf: Fix perf with MT counting other threads")

v4.9.92: Failed to apply! Possible dependencies:
    4e099ca64578 ("MIPS: perf: Use correct VPE ID when setting up VPE tracing")
    e2a7b77d89f6 ("MIPS: perf: Fix perf with MT counting other threads")

v4.4.126: Failed to apply! Possible dependencies:
    4e099ca64578 ("MIPS: perf: Use correct VPE ID when setting up VPE tracing")
    e2a7b77d89f6 ("MIPS: perf: Fix perf with MT counting other threads")


Please let us know if you'd like to have this patch included in a stable tree.

--
Thanks.
Sasha

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-04-05 16:43 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-03 12:31 [PATCH 0/5] MIPS: perf: MT fixes and improvements Matt Redfearn
2018-04-03 12:31 ` Matt Redfearn
2018-04-03 12:31 ` [PATCH 1/5] MIPS: perf: More robustly probe for the presence of per-tc counters Matt Redfearn
2018-04-03 12:31   ` Matt Redfearn
2018-04-05 16:43   ` Sasha Levin
2018-04-03 12:31 ` [PATCH 2/5] MIPS: perf: Use correct VPE ID when setting up VPE tracing Matt Redfearn
2018-04-03 12:31   ` Matt Redfearn
2018-04-03 12:31 ` [PATCH 3/5] MIPS: perf: Fix perf with MT counting other threads Matt Redfearn
2018-04-03 12:31   ` Matt Redfearn
2018-04-03 19:49   ` kbuild test robot
2018-04-03 19:49     ` kbuild test robot
2018-04-05 16:43   ` Sasha Levin
2018-04-03 12:31 ` [PATCH 4/5] MIPS: perf: Allocate per-core counters on demand Matt Redfearn
2018-04-03 12:31   ` Matt Redfearn
2018-04-03 20:34   ` kbuild test robot
2018-04-03 20:34     ` kbuild test robot
2018-04-03 12:31 ` [PATCH 5/5] MIPS: perf: Fold vpe_id() macro into it's one last usage Matt Redfearn
2018-04-03 12:31   ` Matt Redfearn
2018-04-03 20:57   ` kbuild test robot
2018-04-03 20:57     ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.