This series drops 'pmu_lock' usage from all arm platforms which had already
been dropped from arm64 platforms earlier via the following commit.
commit 2a0e2a02e4b7 ("arm64: perf: Remove PMU locking").
Afterwards, drop unused 'pmu_lock' element from 'struct pmu_hw_events'. The
series applies on 6.7-rc1 and has been tested on arm64. Although just build
tested for arm platform.
Cc: Mark Rutland <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Changes in V1:
- Added some build warning fixes
RFC V1:
https://lore.kernel.org/all/[email protected]/
Anshuman Khandual (2):
arm: perf: Remove PMU locking
drivers: perf: arm_pmu: Drop 'pmu_lock' element from 'struct pmu_hw_events'
arch/arm/kernel/perf_event_v6.c | 28 ++++--------------
arch/arm/kernel/perf_event_v7.c | 44 -----------------------------
arch/arm/kernel/perf_event_xscale.c | 44 ++++++-----------------------
drivers/perf/arm_pmu.c | 1 -
include/linux/perf/arm_pmu.h | 6 ----
5 files changed, 13 insertions(+), 110 deletions(-)
--
2.25.1
As 'pmu_lock' element is not being used in any ARM PMU implementation, just
drop this from 'struct pmu_hw_events'.
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anshuman Khandual <[email protected]>
---
drivers/perf/arm_pmu.c | 1 -
include/linux/perf/arm_pmu.h | 6 ------
2 files changed, 7 deletions(-)
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index d712a19e47ac..379479b50bdd 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -893,7 +893,6 @@ struct arm_pmu *armpmu_alloc(void)
struct pmu_hw_events *events;
events = per_cpu_ptr(pmu->hw_events, cpu);
- raw_spin_lock_init(&events->pmu_lock);
events->percpu_pmu = pmu;
}
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 143fbc10ecfe..e2503d48ddee 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -59,12 +59,6 @@ struct pmu_hw_events {
*/
DECLARE_BITMAP(used_mask, ARMPMU_MAX_HWEVENTS);
- /*
- * Hardware lock to serialize accesses to PMU registers. Needed for the
- * read/modify/write sequences.
- */
- raw_spinlock_t pmu_lock;
-
/*
* When using percpu IRQs, we need a percpu dev_id. Place it here as we
* already have to allocate this struct per cpu.
--
2.25.1
The PMU is disabled and enabled, and the counters are programmed from
contexts where interrupts or preemption is disabled.
The functions to toggle the PMU and to program the PMU counters access the
registers directly and don't access data modified by the interrupt handler.
That, and the fact that they're always called from non-preemptible
contexts, means that we don't need to disable interrupts or use a spinlock.
This is very similar to an earlier change on arm64 platform.
commit 2a0e2a02e4b7 ("arm64: perf: Remove PMU locking").
Cc: Mark Rutland <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anshuman Khandual <[email protected]>
---
arch/arm/kernel/perf_event_v6.c | 28 ++++--------------
arch/arm/kernel/perf_event_v7.c | 44 -----------------------------
arch/arm/kernel/perf_event_xscale.c | 44 ++++++-----------------------
3 files changed, 13 insertions(+), 103 deletions(-)
diff --git a/arch/arm/kernel/perf_event_v6.c b/arch/arm/kernel/perf_event_v6.c
index 1ae99deeec54..8fc080c9e4fb 100644
--- a/arch/arm/kernel/perf_event_v6.c
+++ b/arch/arm/kernel/perf_event_v6.c
@@ -268,10 +268,8 @@ static inline void armv6pmu_write_counter(struct perf_event *event, u64 value)
static void armv6pmu_enable_event(struct perf_event *event)
{
- unsigned long val, mask, evt, flags;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
+ unsigned long val, mask, evt;
struct hw_perf_event *hwc = &event->hw;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
int idx = hwc->idx;
if (ARMV6_CYCLE_COUNTER == idx) {
@@ -294,12 +292,10 @@ static void armv6pmu_enable_event(struct perf_event *event)
* Mask out the current event and set the counter to count the event
* that we're interested in.
*/
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = armv6_pmcr_read();
val &= ~mask;
val |= evt;
armv6_pmcr_write(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static irqreturn_t
@@ -362,26 +358,20 @@ armv6pmu_handle_irq(struct arm_pmu *cpu_pmu)
static void armv6pmu_start(struct arm_pmu *cpu_pmu)
{
- unsigned long flags, val;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
+ unsigned long val;
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = armv6_pmcr_read();
val |= ARMV6_PMCR_ENABLE;
armv6_pmcr_write(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void armv6pmu_stop(struct arm_pmu *cpu_pmu)
{
- unsigned long flags, val;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
+ unsigned long val;
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = armv6_pmcr_read();
val &= ~ARMV6_PMCR_ENABLE;
armv6_pmcr_write(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static int
@@ -419,10 +409,8 @@ static void armv6pmu_clear_event_idx(struct pmu_hw_events *cpuc,
static void armv6pmu_disable_event(struct perf_event *event)
{
- unsigned long val, mask, evt, flags;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
+ unsigned long val, mask, evt;
struct hw_perf_event *hwc = &event->hw;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
int idx = hwc->idx;
if (ARMV6_CYCLE_COUNTER == idx) {
@@ -444,20 +432,16 @@ static void armv6pmu_disable_event(struct perf_event *event)
* of ETM bus signal assertion cycles. The external reporting should
* be disabled and so this should never increment.
*/
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = armv6_pmcr_read();
val &= ~mask;
val |= evt;
armv6_pmcr_write(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void armv6mpcore_pmu_disable_event(struct perf_event *event)
{
- unsigned long val, mask, flags, evt = 0;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
+ unsigned long val, mask, evt = 0;
struct hw_perf_event *hwc = &event->hw;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
int idx = hwc->idx;
if (ARMV6_CYCLE_COUNTER == idx) {
@@ -475,12 +459,10 @@ static void armv6mpcore_pmu_disable_event(struct perf_event *event)
* Unlike UP ARMv6, we don't have a way of stopping the counters. We
* simply disable the interrupt reporting.
*/
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = armv6_pmcr_read();
val &= ~mask;
val |= evt;
armv6_pmcr_write(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static int armv6_map_event(struct perf_event *event)
diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
index eb2190477da1..c890354b04e9 100644
--- a/arch/arm/kernel/perf_event_v7.c
+++ b/arch/arm/kernel/perf_event_v7.c
@@ -870,10 +870,8 @@ static void armv7_pmnc_dump_regs(struct arm_pmu *cpu_pmu)
static void armv7pmu_enable_event(struct perf_event *event)
{
- unsigned long flags;
struct hw_perf_event *hwc = &event->hw;
struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
int idx = hwc->idx;
if (!armv7_pmnc_counter_valid(cpu_pmu, idx)) {
@@ -886,7 +884,6 @@ static void armv7pmu_enable_event(struct perf_event *event)
* Enable counter and interrupt, and set the counter to count
* the event that we're interested in.
*/
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
/*
* Disable counter
@@ -910,16 +907,12 @@ static void armv7pmu_enable_event(struct perf_event *event)
* Enable counter
*/
armv7_pmnc_enable_counter(idx);
-
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void armv7pmu_disable_event(struct perf_event *event)
{
- unsigned long flags;
struct hw_perf_event *hwc = &event->hw;
struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
int idx = hwc->idx;
if (!armv7_pmnc_counter_valid(cpu_pmu, idx)) {
@@ -931,7 +924,6 @@ static void armv7pmu_disable_event(struct perf_event *event)
/*
* Disable counter and interrupt
*/
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
/*
* Disable counter
@@ -942,8 +934,6 @@ static void armv7pmu_disable_event(struct perf_event *event)
* Disable interrupt for this counter
*/
armv7_pmnc_disable_intens(idx);
-
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static irqreturn_t armv7pmu_handle_irq(struct arm_pmu *cpu_pmu)
@@ -1009,24 +999,14 @@ static irqreturn_t armv7pmu_handle_irq(struct arm_pmu *cpu_pmu)
static void armv7pmu_start(struct arm_pmu *cpu_pmu)
{
- unsigned long flags;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
-
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
/* Enable all counters */
armv7_pmnc_write(armv7_pmnc_read() | ARMV7_PMNC_E);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void armv7pmu_stop(struct arm_pmu *cpu_pmu)
{
- unsigned long flags;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
-
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
/* Disable all counters */
armv7_pmnc_write(armv7_pmnc_read() & ~ARMV7_PMNC_E);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static int armv7pmu_get_event_idx(struct pmu_hw_events *cpuc,
@@ -1492,14 +1472,10 @@ static void krait_clearpmu(u32 config_base)
static void krait_pmu_disable_event(struct perf_event *event)
{
- unsigned long flags;
struct hw_perf_event *hwc = &event->hw;
int idx = hwc->idx;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
/* Disable counter and interrupt */
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
/* Disable counter */
armv7_pmnc_disable_counter(idx);
@@ -1512,23 +1488,17 @@ static void krait_pmu_disable_event(struct perf_event *event)
/* Disable interrupt for this counter */
armv7_pmnc_disable_intens(idx);
-
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void krait_pmu_enable_event(struct perf_event *event)
{
- unsigned long flags;
struct hw_perf_event *hwc = &event->hw;
int idx = hwc->idx;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
/*
* Enable counter and interrupt, and set the counter to count
* the event that we're interested in.
*/
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
/* Disable counter */
armv7_pmnc_disable_counter(idx);
@@ -1548,8 +1518,6 @@ static void krait_pmu_enable_event(struct perf_event *event)
/* Enable counter */
armv7_pmnc_enable_counter(idx);
-
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void krait_pmu_reset(void *info)
@@ -1825,14 +1793,10 @@ static void scorpion_clearpmu(u32 config_base)
static void scorpion_pmu_disable_event(struct perf_event *event)
{
- unsigned long flags;
struct hw_perf_event *hwc = &event->hw;
int idx = hwc->idx;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
/* Disable counter and interrupt */
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
/* Disable counter */
armv7_pmnc_disable_counter(idx);
@@ -1845,23 +1809,17 @@ static void scorpion_pmu_disable_event(struct perf_event *event)
/* Disable interrupt for this counter */
armv7_pmnc_disable_intens(idx);
-
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void scorpion_pmu_enable_event(struct perf_event *event)
{
- unsigned long flags;
struct hw_perf_event *hwc = &event->hw;
int idx = hwc->idx;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
/*
* Enable counter and interrupt, and set the counter to count
* the event that we're interested in.
*/
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
/* Disable counter */
armv7_pmnc_disable_counter(idx);
@@ -1881,8 +1839,6 @@ static void scorpion_pmu_enable_event(struct perf_event *event)
/* Enable counter */
armv7_pmnc_enable_counter(idx);
-
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void scorpion_pmu_reset(void *info)
diff --git a/arch/arm/kernel/perf_event_xscale.c b/arch/arm/kernel/perf_event_xscale.c
index f6cdcacfb96d..7a2ba1c689a7 100644
--- a/arch/arm/kernel/perf_event_xscale.c
+++ b/arch/arm/kernel/perf_event_xscale.c
@@ -203,10 +203,8 @@ xscale1pmu_handle_irq(struct arm_pmu *cpu_pmu)
static void xscale1pmu_enable_event(struct perf_event *event)
{
- unsigned long val, mask, evt, flags;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
+ unsigned long val, mask, evt;
struct hw_perf_event *hwc = &event->hw;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
int idx = hwc->idx;
switch (idx) {
@@ -229,20 +227,16 @@ static void xscale1pmu_enable_event(struct perf_event *event)
return;
}
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = xscale1pmu_read_pmnc();
val &= ~mask;
val |= evt;
xscale1pmu_write_pmnc(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void xscale1pmu_disable_event(struct perf_event *event)
{
- unsigned long val, mask, evt, flags;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
+ unsigned long val, mask, evt;
struct hw_perf_event *hwc = &event->hw;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
int idx = hwc->idx;
switch (idx) {
@@ -263,12 +257,10 @@ static void xscale1pmu_disable_event(struct perf_event *event)
return;
}
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = xscale1pmu_read_pmnc();
val &= ~mask;
val |= evt;
xscale1pmu_write_pmnc(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static int
@@ -300,26 +292,20 @@ static void xscalepmu_clear_event_idx(struct pmu_hw_events *cpuc,
static void xscale1pmu_start(struct arm_pmu *cpu_pmu)
{
- unsigned long flags, val;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
+ unsigned long val;
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = xscale1pmu_read_pmnc();
val |= XSCALE_PMU_ENABLE;
xscale1pmu_write_pmnc(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void xscale1pmu_stop(struct arm_pmu *cpu_pmu)
{
- unsigned long flags, val;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
+ unsigned long val;
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = xscale1pmu_read_pmnc();
val &= ~XSCALE_PMU_ENABLE;
xscale1pmu_write_pmnc(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static inline u64 xscale1pmu_read_counter(struct perf_event *event)
@@ -549,10 +535,8 @@ xscale2pmu_handle_irq(struct arm_pmu *cpu_pmu)
static void xscale2pmu_enable_event(struct perf_event *event)
{
- unsigned long flags, ien, evtsel;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
+ unsigned long ien, evtsel;
struct hw_perf_event *hwc = &event->hw;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
int idx = hwc->idx;
ien = xscale2pmu_read_int_enable();
@@ -587,18 +571,14 @@ static void xscale2pmu_enable_event(struct perf_event *event)
return;
}
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
xscale2pmu_write_event_select(evtsel);
xscale2pmu_write_int_enable(ien);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void xscale2pmu_disable_event(struct perf_event *event)
{
- unsigned long flags, ien, evtsel, of_flags;
- struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
+ unsigned long ien, evtsel, of_flags;
struct hw_perf_event *hwc = &event->hw;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
int idx = hwc->idx;
ien = xscale2pmu_read_int_enable();
@@ -638,11 +618,9 @@ static void xscale2pmu_disable_event(struct perf_event *event)
return;
}
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
xscale2pmu_write_event_select(evtsel);
xscale2pmu_write_int_enable(ien);
xscale2pmu_write_overflow_flags(of_flags);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static int
@@ -663,26 +641,20 @@ xscale2pmu_get_event_idx(struct pmu_hw_events *cpuc,
static void xscale2pmu_start(struct arm_pmu *cpu_pmu)
{
- unsigned long flags, val;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
+ unsigned long val;
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = xscale2pmu_read_pmnc() & ~XSCALE_PMU_CNT64;
val |= XSCALE_PMU_ENABLE;
xscale2pmu_write_pmnc(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static void xscale2pmu_stop(struct arm_pmu *cpu_pmu)
{
- unsigned long flags, val;
- struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
+ unsigned long val;
- raw_spin_lock_irqsave(&events->pmu_lock, flags);
val = xscale2pmu_read_pmnc();
val &= ~XSCALE_PMU_ENABLE;
xscale2pmu_write_pmnc(val);
- raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
}
static inline u64 xscale2pmu_read_counter(struct perf_event *event)
--
2.25.1
On 11/15/23 14:58, Anshuman Khandual wrote:
> This series drops 'pmu_lock' usage from all arm platforms which had already
> been dropped from arm64 platforms earlier via the following commit.
>
> commit 2a0e2a02e4b7 ("arm64: perf: Remove PMU locking").
>
> Afterwards, drop unused 'pmu_lock' element from 'struct pmu_hw_events'. The
> series applies on 6.7-rc1 and has been tested on arm64. Although just build
> tested for arm platform.
>
> Cc: Mark Rutland <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Changes in V1:
>
> - Added some build warning fixes
Hello Will/Mark,
Any updates or concerns on this series ?
- Anshuman
On Wed, Nov 15, 2023 at 02:58:04PM +0530, Anshuman Khandual wrote:
> The PMU is disabled and enabled, and the counters are programmed from
> contexts where interrupts or preemption is disabled.
>
> The functions to toggle the PMU and to program the PMU counters access the
> registers directly and don't access data modified by the interrupt handler.
> That, and the fact that they're always called from non-preemptible
> contexts, means that we don't need to disable interrupts or use a spinlock.
>
> This is very similar to an earlier change on arm64 platform.
>
> commit 2a0e2a02e4b7 ("arm64: perf: Remove PMU locking").
I realise the commit message is a copy of the wording from 2a0e2a02e4b7, but
some of this isn't quite right; could we please replace that with:
| Currently the 32-bit arm PMU drivers use the pmu_hw_events::lock spinlock in
| their arm_pmu::{start,stop,enable,disable}() callbacks to protect hardware
| state and event data.
|
| This locking is not necessary as the perf core code already provides mutual
| exclusion, disabling interrupts to serialize against the IRQ handler, and
| using perf_event_context::lock to protect against concurrent modifications of
| events cross-cpu.
|
| The locking was removed from the arm64 (now PMUv3) PMU driver in commit:
|
| 2a0e2a02e4b7 ("arm64: perf: Remove PMU locking")
|
| ... and the same reasoning applies to all the 32-bit PMU drivers.
|
| Remove the locking from the 32-bit PMU drivers.
With that wording:
Acked-by: Mark Rutland <[email protected]>
Mark.
> Cc: Mark Rutland <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> arch/arm/kernel/perf_event_v6.c | 28 ++++--------------
> arch/arm/kernel/perf_event_v7.c | 44 -----------------------------
> arch/arm/kernel/perf_event_xscale.c | 44 ++++++-----------------------
> 3 files changed, 13 insertions(+), 103 deletions(-)
>
> diff --git a/arch/arm/kernel/perf_event_v6.c b/arch/arm/kernel/perf_event_v6.c
> index 1ae99deeec54..8fc080c9e4fb 100644
> --- a/arch/arm/kernel/perf_event_v6.c
> +++ b/arch/arm/kernel/perf_event_v6.c
> @@ -268,10 +268,8 @@ static inline void armv6pmu_write_counter(struct perf_event *event, u64 value)
>
> static void armv6pmu_enable_event(struct perf_event *event)
> {
> - unsigned long val, mask, evt, flags;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> + unsigned long val, mask, evt;
> struct hw_perf_event *hwc = &event->hw;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> int idx = hwc->idx;
>
> if (ARMV6_CYCLE_COUNTER == idx) {
> @@ -294,12 +292,10 @@ static void armv6pmu_enable_event(struct perf_event *event)
> * Mask out the current event and set the counter to count the event
> * that we're interested in.
> */
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = armv6_pmcr_read();
> val &= ~mask;
> val |= evt;
> armv6_pmcr_write(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static irqreturn_t
> @@ -362,26 +358,20 @@ armv6pmu_handle_irq(struct arm_pmu *cpu_pmu)
>
> static void armv6pmu_start(struct arm_pmu *cpu_pmu)
> {
> - unsigned long flags, val;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> + unsigned long val;
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = armv6_pmcr_read();
> val |= ARMV6_PMCR_ENABLE;
> armv6_pmcr_write(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void armv6pmu_stop(struct arm_pmu *cpu_pmu)
> {
> - unsigned long flags, val;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> + unsigned long val;
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = armv6_pmcr_read();
> val &= ~ARMV6_PMCR_ENABLE;
> armv6_pmcr_write(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static int
> @@ -419,10 +409,8 @@ static void armv6pmu_clear_event_idx(struct pmu_hw_events *cpuc,
>
> static void armv6pmu_disable_event(struct perf_event *event)
> {
> - unsigned long val, mask, evt, flags;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> + unsigned long val, mask, evt;
> struct hw_perf_event *hwc = &event->hw;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> int idx = hwc->idx;
>
> if (ARMV6_CYCLE_COUNTER == idx) {
> @@ -444,20 +432,16 @@ static void armv6pmu_disable_event(struct perf_event *event)
> * of ETM bus signal assertion cycles. The external reporting should
> * be disabled and so this should never increment.
> */
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = armv6_pmcr_read();
> val &= ~mask;
> val |= evt;
> armv6_pmcr_write(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void armv6mpcore_pmu_disable_event(struct perf_event *event)
> {
> - unsigned long val, mask, flags, evt = 0;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> + unsigned long val, mask, evt = 0;
> struct hw_perf_event *hwc = &event->hw;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> int idx = hwc->idx;
>
> if (ARMV6_CYCLE_COUNTER == idx) {
> @@ -475,12 +459,10 @@ static void armv6mpcore_pmu_disable_event(struct perf_event *event)
> * Unlike UP ARMv6, we don't have a way of stopping the counters. We
> * simply disable the interrupt reporting.
> */
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = armv6_pmcr_read();
> val &= ~mask;
> val |= evt;
> armv6_pmcr_write(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static int armv6_map_event(struct perf_event *event)
> diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
> index eb2190477da1..c890354b04e9 100644
> --- a/arch/arm/kernel/perf_event_v7.c
> +++ b/arch/arm/kernel/perf_event_v7.c
> @@ -870,10 +870,8 @@ static void armv7_pmnc_dump_regs(struct arm_pmu *cpu_pmu)
>
> static void armv7pmu_enable_event(struct perf_event *event)
> {
> - unsigned long flags;
> struct hw_perf_event *hwc = &event->hw;
> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> int idx = hwc->idx;
>
> if (!armv7_pmnc_counter_valid(cpu_pmu, idx)) {
> @@ -886,7 +884,6 @@ static void armv7pmu_enable_event(struct perf_event *event)
> * Enable counter and interrupt, and set the counter to count
> * the event that we're interested in.
> */
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
>
> /*
> * Disable counter
> @@ -910,16 +907,12 @@ static void armv7pmu_enable_event(struct perf_event *event)
> * Enable counter
> */
> armv7_pmnc_enable_counter(idx);
> -
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void armv7pmu_disable_event(struct perf_event *event)
> {
> - unsigned long flags;
> struct hw_perf_event *hwc = &event->hw;
> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> int idx = hwc->idx;
>
> if (!armv7_pmnc_counter_valid(cpu_pmu, idx)) {
> @@ -931,7 +924,6 @@ static void armv7pmu_disable_event(struct perf_event *event)
> /*
> * Disable counter and interrupt
> */
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
>
> /*
> * Disable counter
> @@ -942,8 +934,6 @@ static void armv7pmu_disable_event(struct perf_event *event)
> * Disable interrupt for this counter
> */
> armv7_pmnc_disable_intens(idx);
> -
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static irqreturn_t armv7pmu_handle_irq(struct arm_pmu *cpu_pmu)
> @@ -1009,24 +999,14 @@ static irqreturn_t armv7pmu_handle_irq(struct arm_pmu *cpu_pmu)
>
> static void armv7pmu_start(struct arm_pmu *cpu_pmu)
> {
> - unsigned long flags;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> -
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> /* Enable all counters */
> armv7_pmnc_write(armv7_pmnc_read() | ARMV7_PMNC_E);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void armv7pmu_stop(struct arm_pmu *cpu_pmu)
> {
> - unsigned long flags;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> -
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> /* Disable all counters */
> armv7_pmnc_write(armv7_pmnc_read() & ~ARMV7_PMNC_E);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static int armv7pmu_get_event_idx(struct pmu_hw_events *cpuc,
> @@ -1492,14 +1472,10 @@ static void krait_clearpmu(u32 config_base)
>
> static void krait_pmu_disable_event(struct perf_event *event)
> {
> - unsigned long flags;
> struct hw_perf_event *hwc = &event->hw;
> int idx = hwc->idx;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
>
> /* Disable counter and interrupt */
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
>
> /* Disable counter */
> armv7_pmnc_disable_counter(idx);
> @@ -1512,23 +1488,17 @@ static void krait_pmu_disable_event(struct perf_event *event)
>
> /* Disable interrupt for this counter */
> armv7_pmnc_disable_intens(idx);
> -
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void krait_pmu_enable_event(struct perf_event *event)
> {
> - unsigned long flags;
> struct hw_perf_event *hwc = &event->hw;
> int idx = hwc->idx;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
>
> /*
> * Enable counter and interrupt, and set the counter to count
> * the event that we're interested in.
> */
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
>
> /* Disable counter */
> armv7_pmnc_disable_counter(idx);
> @@ -1548,8 +1518,6 @@ static void krait_pmu_enable_event(struct perf_event *event)
>
> /* Enable counter */
> armv7_pmnc_enable_counter(idx);
> -
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void krait_pmu_reset(void *info)
> @@ -1825,14 +1793,10 @@ static void scorpion_clearpmu(u32 config_base)
>
> static void scorpion_pmu_disable_event(struct perf_event *event)
> {
> - unsigned long flags;
> struct hw_perf_event *hwc = &event->hw;
> int idx = hwc->idx;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
>
> /* Disable counter and interrupt */
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
>
> /* Disable counter */
> armv7_pmnc_disable_counter(idx);
> @@ -1845,23 +1809,17 @@ static void scorpion_pmu_disable_event(struct perf_event *event)
>
> /* Disable interrupt for this counter */
> armv7_pmnc_disable_intens(idx);
> -
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void scorpion_pmu_enable_event(struct perf_event *event)
> {
> - unsigned long flags;
> struct hw_perf_event *hwc = &event->hw;
> int idx = hwc->idx;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
>
> /*
> * Enable counter and interrupt, and set the counter to count
> * the event that we're interested in.
> */
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
>
> /* Disable counter */
> armv7_pmnc_disable_counter(idx);
> @@ -1881,8 +1839,6 @@ static void scorpion_pmu_enable_event(struct perf_event *event)
>
> /* Enable counter */
> armv7_pmnc_enable_counter(idx);
> -
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void scorpion_pmu_reset(void *info)
> diff --git a/arch/arm/kernel/perf_event_xscale.c b/arch/arm/kernel/perf_event_xscale.c
> index f6cdcacfb96d..7a2ba1c689a7 100644
> --- a/arch/arm/kernel/perf_event_xscale.c
> +++ b/arch/arm/kernel/perf_event_xscale.c
> @@ -203,10 +203,8 @@ xscale1pmu_handle_irq(struct arm_pmu *cpu_pmu)
>
> static void xscale1pmu_enable_event(struct perf_event *event)
> {
> - unsigned long val, mask, evt, flags;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> + unsigned long val, mask, evt;
> struct hw_perf_event *hwc = &event->hw;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> int idx = hwc->idx;
>
> switch (idx) {
> @@ -229,20 +227,16 @@ static void xscale1pmu_enable_event(struct perf_event *event)
> return;
> }
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = xscale1pmu_read_pmnc();
> val &= ~mask;
> val |= evt;
> xscale1pmu_write_pmnc(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void xscale1pmu_disable_event(struct perf_event *event)
> {
> - unsigned long val, mask, evt, flags;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> + unsigned long val, mask, evt;
> struct hw_perf_event *hwc = &event->hw;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> int idx = hwc->idx;
>
> switch (idx) {
> @@ -263,12 +257,10 @@ static void xscale1pmu_disable_event(struct perf_event *event)
> return;
> }
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = xscale1pmu_read_pmnc();
> val &= ~mask;
> val |= evt;
> xscale1pmu_write_pmnc(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static int
> @@ -300,26 +292,20 @@ static void xscalepmu_clear_event_idx(struct pmu_hw_events *cpuc,
>
> static void xscale1pmu_start(struct arm_pmu *cpu_pmu)
> {
> - unsigned long flags, val;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> + unsigned long val;
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = xscale1pmu_read_pmnc();
> val |= XSCALE_PMU_ENABLE;
> xscale1pmu_write_pmnc(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void xscale1pmu_stop(struct arm_pmu *cpu_pmu)
> {
> - unsigned long flags, val;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> + unsigned long val;
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = xscale1pmu_read_pmnc();
> val &= ~XSCALE_PMU_ENABLE;
> xscale1pmu_write_pmnc(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static inline u64 xscale1pmu_read_counter(struct perf_event *event)
> @@ -549,10 +535,8 @@ xscale2pmu_handle_irq(struct arm_pmu *cpu_pmu)
>
> static void xscale2pmu_enable_event(struct perf_event *event)
> {
> - unsigned long flags, ien, evtsel;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> + unsigned long ien, evtsel;
> struct hw_perf_event *hwc = &event->hw;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> int idx = hwc->idx;
>
> ien = xscale2pmu_read_int_enable();
> @@ -587,18 +571,14 @@ static void xscale2pmu_enable_event(struct perf_event *event)
> return;
> }
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> xscale2pmu_write_event_select(evtsel);
> xscale2pmu_write_int_enable(ien);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void xscale2pmu_disable_event(struct perf_event *event)
> {
> - unsigned long flags, ien, evtsel, of_flags;
> - struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> + unsigned long ien, evtsel, of_flags;
> struct hw_perf_event *hwc = &event->hw;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> int idx = hwc->idx;
>
> ien = xscale2pmu_read_int_enable();
> @@ -638,11 +618,9 @@ static void xscale2pmu_disable_event(struct perf_event *event)
> return;
> }
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> xscale2pmu_write_event_select(evtsel);
> xscale2pmu_write_int_enable(ien);
> xscale2pmu_write_overflow_flags(of_flags);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static int
> @@ -663,26 +641,20 @@ xscale2pmu_get_event_idx(struct pmu_hw_events *cpuc,
>
> static void xscale2pmu_start(struct arm_pmu *cpu_pmu)
> {
> - unsigned long flags, val;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> + unsigned long val;
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = xscale2pmu_read_pmnc() & ~XSCALE_PMU_CNT64;
> val |= XSCALE_PMU_ENABLE;
> xscale2pmu_write_pmnc(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static void xscale2pmu_stop(struct arm_pmu *cpu_pmu)
> {
> - unsigned long flags, val;
> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
> + unsigned long val;
>
> - raw_spin_lock_irqsave(&events->pmu_lock, flags);
> val = xscale2pmu_read_pmnc();
> val &= ~XSCALE_PMU_ENABLE;
> xscale2pmu_write_pmnc(val);
> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
> }
>
> static inline u64 xscale2pmu_read_counter(struct perf_event *event)
> --
> 2.25.1
>
On Wed, Nov 15, 2023 at 02:58:05PM +0530, Anshuman Khandual wrote:
> As 'pmu_lock' element is not being used in any ARM PMU implementation, just
> drop this from 'struct pmu_hw_events'.
>
> Cc: Will Deacon <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Anshuman Khandual <[email protected]>
Acked-by: Mark Rutland <[email protected]>
Mark.
> ---
> drivers/perf/arm_pmu.c | 1 -
> include/linux/perf/arm_pmu.h | 6 ------
> 2 files changed, 7 deletions(-)
>
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index d712a19e47ac..379479b50bdd 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -893,7 +893,6 @@ struct arm_pmu *armpmu_alloc(void)
> struct pmu_hw_events *events;
>
> events = per_cpu_ptr(pmu->hw_events, cpu);
> - raw_spin_lock_init(&events->pmu_lock);
> events->percpu_pmu = pmu;
> }
>
> diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
> index 143fbc10ecfe..e2503d48ddee 100644
> --- a/include/linux/perf/arm_pmu.h
> +++ b/include/linux/perf/arm_pmu.h
> @@ -59,12 +59,6 @@ struct pmu_hw_events {
> */
> DECLARE_BITMAP(used_mask, ARMPMU_MAX_HWEVENTS);
>
> - /*
> - * Hardware lock to serialize accesses to PMU registers. Needed for the
> - * read/modify/write sequences.
> - */
> - raw_spinlock_t pmu_lock;
> -
> /*
> * When using percpu IRQs, we need a percpu dev_id. Place it here as we
> * already have to allocate this struct per cpu.
> --
> 2.25.1
>
On Tue, Nov 28, 2023 at 10:59:49AM +0530, Anshuman Khandual wrote:
>
>
> On 11/15/23 14:58, Anshuman Khandual wrote:
> > This series drops 'pmu_lock' usage from all arm platforms which had already
> > been dropped from arm64 platforms earlier via the following commit.
> >
> > commit 2a0e2a02e4b7 ("arm64: perf: Remove PMU locking").
> >
> > Afterwards, drop unused 'pmu_lock' element from 'struct pmu_hw_events'. The
> > series applies on 6.7-rc1 and has been tested on arm64. Although just build
> > tested for arm platform.
> >
> > Cc: Mark Rutland <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Cc: Russell King <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> >
> > Changes in V1:
> >
> > - Added some build warning fixes
>
> Hello Will/Mark,
>
> Any updates or concerns on this series ?
The changes themselves look good to me. I'd like to tweak the commit message
for the first patch (and I've given specific wording that we can use).
Will, are you happy to fold that in when applying, or would you prefer that
Anshuman sends a v2?
Mark.
On Wed, 15 Nov 2023 14:58:03 +0530, Anshuman Khandual wrote:
> This series drops 'pmu_lock' usage from all arm platforms which had already
> been dropped from arm64 platforms earlier via the following commit.
>
> commit 2a0e2a02e4b7 ("arm64: perf: Remove PMU locking").
>
> Afterwards, drop unused 'pmu_lock' element from 'struct pmu_hw_events'. The
> series applies on 6.7-rc1 and has been tested on arm64. Although just build
> tested for arm platform.
>
> [...]
Applied to will (for-next/perf), thanks!
[1/2] arm: perf: Remove PMU locking
https://git.kernel.org/will/c/5cd7da19cb97
[2/2] drivers: perf: arm_pmu: Drop 'pmu_lock' element from 'struct pmu_hw_events'
https://git.kernel.org/will/c/118eb89b1e7f
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev