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

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

On 24/03/15 09:13, Wanpeng Li wrote:
> Hi Juri,
> On Tue, Mar 24, 2015 at 09:27:09AM +0000, Juri Lelli wrote:
>> 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.
> 
> I remember you mentioned that there is a bug through IRC last week, if this 
> patch solve it?
>

It seems to fix it. With the previous check we correctly fail to turn
off a cpu with -dl task running only the first time. After that the
bandwidth information associated with it was gone and subsequent hotplug
operations on the same cpu would turn it off.

Thanks,

- Juri

> Regards,
> Wanpeng Li 
> 
>>
>> 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
>