On 19 June 2014 11:47, Alex Shi alex.shi@linaro.org wrote:
On 06/18/2014 09:18 PM, Jon Medhurst (Tixy) wrote:
On Wed, 2014-06-18 at 20:50 +0800, Alex Shi wrote:
when lock checking option enabled in kernel config, TC2 catch a deadlock bug as following,
This was reported last month by Naresh and was discussed again when I found the problem when testing this month's HMP patches from ARM, so this is somewhat old news. (You we're on CC to all those emails too)
Yes, but is a very severe bug for HMP. It could bring much trouble to LSK user. And it looks not too hard to be resolved in time. So I try to raise this issue again. :)
It looks hmp_idle_pull --> hmp_keepalive_delay --> cpuidle_driver_ref interrupt into cpuidle_register_driver.
Since what we needs of cpuidle_driver in hmp_keepalive_delay are exit_latency/target_residency, they should be fixed for this board.
So, could we avoid the cpuidle_driver_ref calling by giving a fixed value?
Where do we get the fixed value from? These are properties of cpuidle, and as cpuidle is not integrated into the scheduler, asking the cpuidle driver for them seems like the reasonable thing to do. Though if they are fixed and not dependent on current state, then perhaps the idle driver can be hacked to call into the scheduler at probe time to provide the values for the scheduler to stash somewhere
Chris did say he was going to look at a solution, I don't know what his thoughts are.
To avoid the lock using, could we try per_cpu cpuidle driver?
Naresh, could you like to test this patch?
Sure Alex.
I will test and post you results.
Thanks -Naresh
From c938115997899014148b401ab91ce67fb94a0f51 Mon Sep 17 00:00:00 2001 From: Alex Shi alex.shi@linaro.org Date: Thu, 19 Jun 2014 13:47:35 +0800 Subject: [PATCH] HMP: use per cpu cpuidle driver to fix deadlock in hmp_idle_pull
Using per cpu cpuidle driver to fix deadlock in hmp_idle_pull. Otherwise a deadlock happened when do bl_idle_init.
[ 113.878664] other info that might help us debug this: [ 113.878667] Possible unsafe locking scenario: [ 113.878667] [ 113.878670] CPU0 [ 113.878673] ---- [ 113.878681] lock(cpuidle_driver_lock); [ 113.878684] <Interrupt> [ 113.878691] lock(cpuidle_driver_lock); [ 113.878693] [ 113.878693] *** DEADLOCK *** [ 113.878693] [ 113.878697] 1 lock held by ksoftirqd/4/28: [ 113.878719] #0: (hmp_force_migration){+.....}, at: [<c0054da5>] hmp_idle_pull+0x49/0x508
Signed-off-by: Alex Shi alex.shi@linaro.org
kernel/sched/fair.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 71da724..25c6162 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -3587,10 +3587,12 @@ static enum hrtimer_restart hmp_cpu_keepalive_notify(struct hrtimer *hrtimer)
- If there are any, set ns_delay to
- ('target_residency of state with shortest too-big latency' - 1) * 1000.
*/ -static void hmp_keepalive_delay(unsigned int *ns_delay) +static void hmp_keepalive_delay(int cpu, unsigned int *ns_delay) {
struct cpuidle_device *dev = per_cpu(cpuidle_devices, cpu); struct cpuidle_driver *drv;
drv = cpuidle_driver_ref();
drv = cpuidle_get_cpu_driver(dev); if (drv) { unsigned int us_delay = UINT_MAX; unsigned int us_max_delay = *ns_delay / 1000;@@ -3623,7 +3625,7 @@ static void hmp_cpu_keepalive_trigger(void) CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED); keepalive->timer.function = hmp_cpu_keepalive_notify;
hmp_keepalive_delay(&ns_delay);
hmp_keepalive_delay(cpu, &ns_delay); keepalive->delay = ns_to_ktime(ns_delay); keepalive->init = true; }-- 1.9.1