From: khilman@baylibre•com (Kevin Hilman)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] drivers/perf: arm-pmu: fix RCU usage on resume from idle states
Date: Tue, 19 Apr 2016 14:48:40 -0700 [thread overview]
Message-ID: <7hr3e1b77r.fsf@baylibre.com> (raw)
In-Reply-To: <20160419172320.GD4509@arm.com> (Will Deacon's message of "Tue, 19 Apr 2016 18:23:20 +0100")
Will Deacon <will.deacon@arm•com> writes:
> Hi Lorenzo,
>
> On Tue, Apr 19, 2016 at 06:08:09PM +0100, Lorenzo Pieralisi wrote:
>> Commit da4e4f18afe0 ("drivers/perf: arm_pmu: implement CPU_PM notifier")
>> added code in the arm perf infrastructure that allows the kernel to
>> save/restore perf counters whenever the CPU enters a low-power idle
>> state. The kernel saves/restores the counters for each active event
>> through the armpmu_{stop/start} ARM pmu API, so that the idle state
>> enter/exit power cycle is emulated through pmu start/stop operations
>> for each event in use.
>>
>> However, calling armpmu_start() for each active event on power up
>> executes code that requires RCU locking (perf_event_update_userpage())
>> to be functional, so, given that the core may call the CPU_PM notifiers
>> while running the idle thread in an quiescent RCU state this is not
>> allowed as detected through the following splat when kernel is run with
>> CONFIG_PROVE_LOCKING enabled:
>>
>> [ 49.293286]
>> [ 49.294761] ===============================
>> [ 49.298895] [ INFO: suspicious RCU usage. ]
>> [ 49.303031] 4.6.0-rc3+ #421 Not tainted
>> [ 49.306821] -------------------------------
>> [ 49.310956] include/linux/rcupdate.h:872 rcu_read_lock() used
>> illegally while idle!
>> [ 49.318530]
>> [ 49.318530] other info that might help us debug this:
>> [ 49.318530]
>> [ 49.326451]
>> [ 49.326451] RCU used illegally from idle CPU!
>> [ 49.326451] rcu_scheduler_active = 1, debug_locks = 0
>> [ 49.337209] RCU used illegally from extended quiescent state!
>> [ 49.342892] 2 locks held by swapper/2/0:
>> [ 49.346768] #0: (cpu_pm_notifier_lock){......}, at:
>> [<ffffff8008163c28>] cpu_pm_exit+0x18/0x80
>> [ 49.355492] #1: (rcu_read_lock){......}, at: [<ffffff800816dc38>]
>> perf_event_update_userpage+0x0/0x260
>>
>> This patch refactors the perf CPU_PM notifiers to add a boolean
>> flag to the function updating the counters event period, so that the
>> userpage update can be skipped when resuming from low-power whilst
>> keeping correct save/restore functionality for the running events.
>>
>> As a side effect the kernel, while resuming from low-power with
>> perf events enabled, runs with a userspace view of active counters that
>> is not up-to-date with the kernel one, but since the core power down is
>> not really a PMU event start/stop this can be considered acceptable and
>> the userspace event snapshot will update the user view of counters
>> on subsequent perf event updates requested by either the perf API
>> or event counters overflow-triggered interrupts.
>>
>> Fixes: da4e4f18afe0 ("drivers/perf: arm_pmu: implement CPU_PM notifier")
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm•com>
>> Reported-by: James Morse <james.morse@arm•com>
>> Cc: Ashwin Chaugule <ashwin.chaugule@linaro•org>
>> Cc: Will Deacon <will.deacon@arm•com>
>> Cc: Kevin Hilman <khilman@baylibre•com>
>> Cc: Sudeep Holla <sudeep.holla@arm•com>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro•org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro•org>
>> Cc: Mark Rutland <mark.rutland@arm•com>
>> ---
>> drivers/perf/arm_pmu.c | 26 +++++++++++++++++++++-----
>> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> This is horrible, but I think it's the best we can do without completely
> redesigning the way in which we save/restore the PMU state. We should do
> that, but not for 4.6!
>
> Acked-by: Will Deacon <will.deacon@arm•com>
Maybe RCU_NONIDLE() will help here?
Kevin
next prev parent reply other threads:[~2016-04-19 21:48 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-19 17:08 [PATCH] drivers/perf: arm-pmu: fix RCU usage on resume from idle states Lorenzo Pieralisi
2016-04-19 17:23 ` Will Deacon
2016-04-19 21:48 ` Kevin Hilman [this message]
2016-04-20 13:41 ` Lorenzo Pieralisi
2016-04-20 15:45 ` Catalin Marinas
2016-04-20 16:23 ` Paul E. McKenney
2016-04-20 16:52 ` Lorenzo Pieralisi
2016-04-20 17:19 ` Paul E. McKenney
2016-04-20 10:48 ` Catalin Marinas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7hr3e1b77r.fsf@baylibre.com \
--to=khilman@baylibre$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox