Re: [PATCH RESEND v10] sched/deadline: support dl task migration during cpu hotplug

[Juri Lelli <juri.lelli@arm.com>]

Hi,

On 23/03/2015 08:55, Peter Zijlstra wrote:
> On Mon, Mar 23, 2015 at 08:25:04AM +0100, Ingo Molnar wrote:
>>>>>>>>> +			if (cpu >= nr_cpu_ids) {
>>>>>>>>> +				if (dl_bandwidth_enabled()) {
>>>>>>>>> +					/*
>>>>>>>>> +					 * Fail to find any suitable cpu.
>>>>>>>>> +					 * The task will never come back!
>>>>>>>>> +					 */
>>>>>>>>> +					WARN_ON(1);
>>>>>>>>
>>>>>>>> Can this condition happen to users with a non-buggy kernel?
> 
>> I still haven't seen a satisfactory answer to this question. Please 
>> don't resend patches without clearing questions raised during review.
> 
> So I had a look on Friday, it _should_ not happen, but it does due to a
> second bug Juri is currently chasing down.
> 

Right, it should not happen. It happens because hotplug operations are
destructive w.r.t. cpusets. Peter, how about we move the check you put
in sched_cpu_inactive() to cpuset_cpu_inactive()? This way, if we fail,
we don't need to destroy/rebuild the domains.

Thanks,

- Juri

>From 65e8033e05f8b70116747062d00d5a5c266699fb Mon Sep 17 00:00:00 2001
From: Juri Lelli <juri.lelli@gmail.com>
Date: Tue, 24 Mar 2015 07:47:03 +0000
Subject: [PATCH] sched/core: check for available -dl bandwidth in
 cpuset_cpu_inactive

Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
 kernel/sched/core.c | 56 ++++++++++++++++++++++++++---------------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 50927eb..3723ad0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5318,36 +5318,13 @@ static int sched_cpu_active(struct notifier_block *nfb,
 static int sched_cpu_inactive(struct notifier_block *nfb,
 					unsigned long action, void *hcpu)
 {
-	unsigned long flags;
-	long cpu = (long)hcpu;
-	struct dl_bw *dl_b;
-
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_DOWN_PREPARE:
-		set_cpu_active(cpu, false);
-
-		/* explicitly allow suspend */
-		if (!(action & CPU_TASKS_FROZEN)) {
-			bool overflow;
-			int cpus;
-
-			rcu_read_lock_sched();
-			dl_b = dl_bw_of(cpu);
-
-			raw_spin_lock_irqsave(&dl_b->lock, flags);
-			cpus = dl_bw_cpus(cpu);
-			overflow = __dl_overflow(dl_b, cpus, 0, 0);
-			raw_spin_unlock_irqrestore(&dl_b->lock, flags);
-
-			rcu_read_unlock_sched();
-
-			if (overflow)
-				return notifier_from_errno(-EBUSY);
-		}
+		set_cpu_active((long)hcpu, false);
 		return NOTIFY_OK;
+	default:
+		return NOTIFY_DONE;
 	}
-
-	return NOTIFY_DONE;
 }
 
 static int __init migration_init(void)
@@ -7001,7 +6978,6 @@ static int cpuset_cpu_active(struct notifier_block *nfb, unsigned long action,
 		 */
 
 	case CPU_ONLINE:
-	case CPU_DOWN_FAILED:
 		cpuset_update_active_cpus(true);
 		break;
 	default:
@@ -7013,8 +6989,32 @@ static int cpuset_cpu_active(struct notifier_block *nfb, unsigned long action,
 static int cpuset_cpu_inactive(struct notifier_block *nfb, unsigned long action,
 			       void *hcpu)
 {
-	switch (action) {
+	unsigned long flags;
+	long cpu = (long)hcpu;
+	struct dl_bw *dl_b;
+
+	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_DOWN_PREPARE:
+		/* explicitly allow suspend */
+		if (!(action & CPU_TASKS_FROZEN)) {
+			bool overflow;
+			int cpus;
+
+			rcu_read_lock_sched();
+			dl_b = dl_bw_of(cpu);
+
+			raw_spin_lock_irqsave(&dl_b->lock, flags);
+			cpus = dl_bw_cpus(cpu);
+			overflow = __dl_overflow(dl_b, cpus, 0, 0);
+			raw_spin_unlock_irqrestore(&dl_b->lock, flags);
+
+			rcu_read_unlock_sched();
+
+			if (overflow) {
+				trace_printk("hotplug failed for cpu %lu", cpu);
+				return notifier_from_errno(-EBUSY);
+			}
+		}
 		cpuset_update_active_cpus(false);
 		break;
 	case CPU_DOWN_PREPARE_FROZEN:
-- 
2.3.0