lkml.org 
[lkml]   [2016]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[RFC][PATCH] perf: Remove superfluous perf_pmu_disable calls

Hai,

So pmu::add() and pmu::del() are guaranteed to be called with ctx->lock
held, which implies that local IRQs are disabled.

Furthermore, it is also guaranteed that perf_pmu_disable() is already
called when we call these methods.

The following patch removes all perf_pmu_{dis,en}able() calls (and
local_irq_disable() where encountered) from pmu::{add,del}()
implementations.

pmu::{start,stop}() are a little bit tricky, since we can call them from
the PMI handler, but we do guarantee local IRQs are disabled. PPC in
particular seems to need perf_pmu_{dis,en}able() there to actually
reprogram things, this is safe for them since they don't have actual
NMIs I suppose.

---
arch/alpha/kernel/perf_event.c | 22 ----------------------
arch/mips/kernel/perf_event_mipsxx.c | 3 ---
arch/powerpc/perf/core-book3s.c | 16 +++-------------
arch/powerpc/perf/core-fsl-emb.c | 7 +++----
arch/s390/kernel/perf_cpum_sf.c | 4 ----
arch/sh/kernel/perf_event.c | 3 ---
b/arch/blackfin/kernel/perf_event.c | 3 ---
b/arch/metag/kernel/perf/perf_event.c | 3 ---
b/arch/sparc/kernel/perf_event.c | 9 ---------
drivers/bus/arm-cci.c | 3 ---
drivers/perf/arm_pmu.c | 3 ---
kernel/events/core.c | 6 ++++++
12 files changed, 12 insertions(+), 70 deletions(-)

--- a/arch/alpha/kernel/perf_event.c
+++ b/arch/alpha/kernel/perf_event.c
@@ -435,18 +435,6 @@ static int alpha_pmu_add(struct perf_eve
struct hw_perf_event *hwc = &event->hw;
int n0;
int ret;
- unsigned long irq_flags;
-
- /*
- * The Sparc code has the IRQ disable first followed by the perf
- * disable, however this can lead to an overflowed counter with the
- * PMI disabled on rare occasions. The alpha_perf_event_update()
- * routine should detect this situation by noting a negative delta,
- * nevertheless we disable the PMCs first to enable a potential
- * final PMI to occur before we disable interrupts.
- */
- perf_pmu_disable(event->pmu);
- local_irq_save(irq_flags);

/* Default to error to be returned */
ret = -EAGAIN;
@@ -469,9 +457,6 @@ static int alpha_pmu_add(struct perf_eve
if (!(flags & PERF_EF_START))
hwc->state |= PERF_HES_STOPPED;

- local_irq_restore(irq_flags);
- perf_pmu_enable(event->pmu);
-
return ret;
}

@@ -485,12 +470,8 @@ static void alpha_pmu_del(struct perf_ev
{
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
struct hw_perf_event *hwc = &event->hw;
- unsigned long irq_flags;
int j;

- perf_pmu_disable(event->pmu);
- local_irq_save(irq_flags);
-
for (j = 0; j < cpuc->n_events; j++) {
if (event == cpuc->event[j]) {
int idx = cpuc->current_idx[j];
@@ -514,9 +495,6 @@ static void alpha_pmu_del(struct perf_ev
break;
}
}
-
- local_irq_restore(irq_flags);
- perf_pmu_enable(event->pmu);
}


--- a/arch/blackfin/kernel/perf_event.c
+++ b/arch/blackfin/kernel/perf_event.c
@@ -350,8 +350,6 @@ static int bfin_pmu_add(struct perf_even
int idx = hwc->idx;
int ret = -EAGAIN;

- perf_pmu_disable(event->pmu);
-
if (__test_and_set_bit(idx, cpuc->used_mask)) {
idx = find_first_zero_bit(cpuc->used_mask, MAX_HWEVENTS);
if (idx == MAX_HWEVENTS)
@@ -370,7 +368,6 @@ static int bfin_pmu_add(struct perf_even
perf_event_update_userpage(event);
ret = 0;
out:
- perf_pmu_enable(event->pmu);
return ret;
}

--- a/arch/metag/kernel/perf/perf_event.c
+++ b/arch/metag/kernel/perf/perf_event.c
@@ -310,8 +310,6 @@ static int metag_pmu_add(struct perf_eve
struct hw_perf_event *hwc = &event->hw;
int idx = 0, ret = 0;

- perf_pmu_disable(event->pmu);
-
/* check whether we're counting instructions */
if (hwc->config == 0x100) {
if (__test_and_set_bit(METAG_INST_COUNTER,
@@ -342,7 +340,6 @@ static int metag_pmu_add(struct perf_eve

perf_event_update_userpage(event);
out:
- perf_pmu_enable(event->pmu);
return ret;
}

--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -463,8 +463,6 @@ static int mipspmu_add(struct perf_event
int idx;
int err = 0;

- perf_pmu_disable(event->pmu);
-
/* To look for a free counter for this event. */
idx = mipsxx_pmu_alloc_counter(cpuc, hwc);
if (idx < 0) {
@@ -488,7 +486,6 @@ static int mipspmu_add(struct perf_event
perf_event_update_userpage(event);

out:
- perf_pmu_enable(event->pmu);
return err;
}

--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1406,13 +1406,9 @@ static int collect_events(struct perf_ev
static int power_pmu_add(struct perf_event *event, int ef_flags)
{
struct cpu_hw_events *cpuhw;
- unsigned long flags;
int n0;
int ret = -EAGAIN;

- local_irq_save(flags);
- perf_pmu_disable(event->pmu);
-
/*
* Add the event to the list (if there is room)
* and check whether the total set is still feasible.
@@ -1464,8 +1460,6 @@ static int power_pmu_add(struct perf_eve
event->attr.branch_sample_type);
}

- perf_pmu_enable(event->pmu);
- local_irq_restore(flags);
return ret;
}

@@ -1476,10 +1470,6 @@ static void power_pmu_del(struct perf_ev
{
struct cpu_hw_events *cpuhw;
long i;
- unsigned long flags;
-
- local_irq_save(flags);
- perf_pmu_disable(event->pmu);

power_pmu_read(event);

@@ -1518,9 +1508,6 @@ static void power_pmu_del(struct perf_ev

if (has_branch_stack(event))
power_pmu_bhrb_disable(event);
-
- perf_pmu_enable(event->pmu);
- local_irq_restore(flags);
}

/*
@@ -1543,6 +1530,9 @@ static void power_pmu_start(struct perf_
if (ef_flags & PERF_EF_RELOAD)
WARN_ON_ONCE(!(event->hw.state & PERF_HES_UPTODATE));

+ /*
+ * XXX do we really need these? If so, comment please.
+ */
local_irq_save(flags);
perf_pmu_disable(event->pmu);

--- a/arch/powerpc/perf/core-fsl-emb.c
+++ b/arch/powerpc/perf/core-fsl-emb.c
@@ -298,7 +298,6 @@ static int fsl_emb_pmu_add(struct perf_e
u64 val;
int i;

- perf_pmu_disable(event->pmu);
cpuhw = &get_cpu_var(cpu_hw_events);

if (event->hw.config & FSL_EMB_EVENT_RESTRICTED)
@@ -346,7 +345,6 @@ static int fsl_emb_pmu_add(struct perf_e
ret = 0;
out:
put_cpu_var(cpu_hw_events);
- perf_pmu_enable(event->pmu);
return ret;
}

@@ -356,7 +354,6 @@ static void fsl_emb_pmu_del(struct perf_
struct cpu_hw_events *cpuhw;
int i = event->hw.idx;

- perf_pmu_disable(event->pmu);
if (i < 0)
goto out;

@@ -384,7 +381,6 @@ static void fsl_emb_pmu_del(struct perf_
cpuhw->n_events--;

out:
- perf_pmu_enable(event->pmu);
put_cpu_var(cpu_hw_events);
}

@@ -403,6 +399,9 @@ static void fsl_emb_pmu_start(struct per
if (ef_flags & PERF_EF_RELOAD)
WARN_ON_ONCE(!(event->hw.state & PERF_HES_UPTODATE));

+ /*
+ * XXX do we really need these, if so comment please.
+ */
local_irq_save(flags);
perf_pmu_disable(event->pmu);

--- a/arch/s390/kernel/perf_cpum_sf.c
+++ b/arch/s390/kernel/perf_cpum_sf.c
@@ -1358,7 +1358,6 @@ static int cpumsf_pmu_add(struct perf_ev
return -EINVAL;

err = 0;
- perf_pmu_disable(event->pmu);

event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;

@@ -1392,7 +1391,6 @@ static int cpumsf_pmu_add(struct perf_ev
cpumsf_pmu_start(event, PERF_EF_RELOAD);
out:
perf_event_update_userpage(event);
- perf_pmu_enable(event->pmu);
return err;
}

@@ -1400,7 +1398,6 @@ static void cpumsf_pmu_del(struct perf_e
{
struct cpu_hw_sf *cpuhw = this_cpu_ptr(&cpu_hw_sf);

- perf_pmu_disable(event->pmu);
cpumsf_pmu_stop(event, PERF_EF_UPDATE);

cpuhw->lsctl.es = 0;
@@ -1409,7 +1406,6 @@ static void cpumsf_pmu_del(struct perf_e
cpuhw->event = NULL;

perf_event_update_userpage(event);
- perf_pmu_enable(event->pmu);
}

CPUMF_EVENT_ATTR(SF, SF_CYCLES_BASIC, PERF_EVENT_CPUM_SF);
--- a/arch/sh/kernel/perf_event.c
+++ b/arch/sh/kernel/perf_event.c
@@ -269,8 +269,6 @@ static int sh_pmu_add(struct perf_event
int idx = hwc->idx;
int ret = -EAGAIN;

- perf_pmu_disable(event->pmu);
-
if (__test_and_set_bit(idx, cpuc->used_mask)) {
idx = find_first_zero_bit(cpuc->used_mask, sh_pmu->num_events);
if (idx == sh_pmu->num_events)
@@ -289,7 +287,6 @@ static int sh_pmu_add(struct perf_event
perf_event_update_userpage(event);
ret = 0;
out:
- perf_pmu_enable(event->pmu);
return ret;
}

--- a/arch/sparc/kernel/perf_event.c
+++ b/arch/sparc/kernel/perf_event.c
@@ -1099,11 +1099,8 @@ static void sparc_pmu_stop(struct perf_e
static void sparc_pmu_del(struct perf_event *event, int _flags)
{
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
- unsigned long flags;
int i;

- local_irq_save(flags);
-
for (i = 0; i < cpuc->n_events; i++) {
if (event == cpuc->event[i]) {
/* Absorb the final count and turn off the
@@ -1127,8 +1124,6 @@ static void sparc_pmu_del(struct perf_ev
break;
}
}
-
- local_irq_restore(flags);
}

static void sparc_pmu_read(struct perf_event *event)
@@ -1358,9 +1353,6 @@ static int sparc_pmu_add(struct perf_eve
{
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
int n0, ret = -EAGAIN;
- unsigned long flags;
-
- local_irq_save(flags);

n0 = cpuc->n_events;
if (n0 >= sparc_pmu->max_hw_events)
@@ -1393,7 +1385,6 @@ static int sparc_pmu_add(struct perf_eve

ret = 0;
out:
- local_irq_restore(flags);
return ret;
}

--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -1230,8 +1230,6 @@ static int cci_pmu_add(struct perf_event
int idx;
int err = 0;

- perf_pmu_disable(event->pmu);
-
/* If we don't have a space for the counter then finish early. */
idx = pmu_get_event_idx(hw_events, event);
if (idx < 0) {
@@ -1250,7 +1248,6 @@ static int cci_pmu_add(struct perf_event
perf_event_update_userpage(event);

out:
- perf_pmu_enable(event->pmu);
return err;
}

--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -240,8 +240,6 @@ armpmu_add(struct perf_event *event, int
if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
return -ENOENT;

- perf_pmu_disable(event->pmu);
-
/* If we don't have a space for the counter then finish early. */
idx = armpmu->get_event_idx(hw_events, event);
if (idx < 0) {
@@ -265,7 +263,6 @@ armpmu_add(struct perf_event *event, int
perf_event_update_userpage(event);

out:
- perf_pmu_enable(event->pmu);
return err;
}

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8597,6 +8597,12 @@ int perf_pmu_register(struct pmu *pmu, c
}
}

+ /*
+ * Software events cannot have pmu_{en,dis}able() calls because that
+ * would make it 'hard' to put them in groups with hardware events.
+ */
+ WARN_ON_ONCE(pmu->task_ctx_nr == perf_sw_event && pmu->pmu_enable);
+
if (!pmu->pmu_enable) {
pmu->pmu_enable = perf_pmu_nop_void;
pmu->pmu_disable = perf_pmu_nop_void;
\
 
 \ /
  Last update: 2016-07-06 17:01    [W:0.029 / U:21.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site