* [PATCH v4] sched/deadline: support dl task migration during cpu hotplug
@ 2014-11-11 2:30 Wanpeng Li
2014-11-11 11:10 ` Kirill Tkhai
0 siblings, 1 reply; 6+ messages in thread
From: Wanpeng Li @ 2014-11-11 2:30 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Juri Lelli
Cc: Kirill Tkhai, linux-kernel, Wanpeng Li
I observe that dl task can't be migrated to other cpus during cpu hotplug, in
addition, task may/may not be running again if cpu is added back. The root cause
which I found is that dl task will be throtted and removed from dl rq after
comsuming all budget, which leads to stop task can't pick it up from dl rq and
migrate to other cpus during hotplug.
The method to reproduce:
schedtool -E -t 50000:100000 -e ./test
Actually test is just a simple for loop. Then observe which cpu the test
task is on.
echo 0 > /sys/devices/system/cpu/cpuN/online
This patch fix it by push the task to another cpu in dl_task_timer() if
rq is offline.
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
v3 -> v4:
* use tsk_cpus_allowed wrapper
* fix compile error
v2 -> v3:
* don't get_task_struct
* if cannot preempt any rq, fallback to pick any online cpus
* use cpu_active_mask as original later_mask if cpu is offline
v1 -> v2:
* push the task to another cpu in dl_task_timer() if rq is offline.
kernel/sched/deadline.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 00324af..e0fbba4 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -487,6 +487,7 @@ static int start_dl_timer(struct sched_dl_entity *dl_se, bool boosted)
return hrtimer_active(&dl_se->dl_timer);
}
+static struct rq *find_lock_later_rq(struct task_struct *task, struct rq *rq);
/*
* This is the bandwidth enforcement timer callback. If here, we know
* a task is not on its dl_rq, since the fact that the timer was running
@@ -538,6 +539,46 @@ again:
update_rq_clock(rq);
dl_se->dl_throttled = 0;
dl_se->dl_yielded = 0;
+
+ /*
+ * So if we find that the rq the task was on is no longer
+ * available, we need to select a new rq.
+ */
+ if (!rq->online) {
+ struct rq *later_rq = NULL;
+
+ raw_spin_unlock(&rq->lock);
+
+ later_rq = find_lock_later_rq(p, rq);
+
+ if (!later_rq) {
+ int cpu;
+
+ /*
+ * If cannot preempt any rq, fallback to pick any
+ * online cpu.
+ */
+ for_each_cpu(cpu, tsk_cpus_allowed(p))
+ if (cpu_online(cpu))
+ later_rq = cpu_rq(cpu);
+ if (!later_rq) {
+ pr_warn("fail to find any online and task "
+ "will never come back to us\n");
+ goto out;
+ }
+ }
+
+ deactivate_task(rq, p, 0);
+ set_task_cpu(p, later_rq->cpu);
+ activate_task(later_rq, p, 0);
+
+ resched_curr(later_rq);
+
+ double_unlock_balance(rq, later_rq);
+
+ goto out;
+ }
+
if (task_on_rq_queued(p)) {
enqueue_task_dl(rq, p, ENQUEUE_REPLENISH);
if (dl_task(rq->curr))
@@ -555,7 +596,7 @@ again:
}
unlock:
raw_spin_unlock(&rq->lock);
-
+out:
return HRTIMER_NORESTART;
}
@@ -1185,8 +1226,12 @@ static int find_later_rq(struct task_struct *task)
* We have to consider system topology and task affinity
* first, then we can look for a suitable cpu.
*/
- cpumask_copy(later_mask, task_rq(task)->rd->span);
- cpumask_and(later_mask, later_mask, cpu_active_mask);
+ if (likely(task_rq(task)->online)) {
+ cpumask_copy(later_mask, task_rq(task)->rd->span);
+ cpumask_and(later_mask, later_mask, cpu_active_mask);
+ } else
+ /* for offline cpus we have a singleton rd */
+ cpumask_copy(later_mask, cpu_active_mask);
cpumask_and(later_mask, later_mask, &task->cpus_allowed);
best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
task, later_mask);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4] sched/deadline: support dl task migration during cpu hotplug
2014-11-11 2:30 [PATCH v4] sched/deadline: support dl task migration during cpu hotplug Wanpeng Li
@ 2014-11-11 11:10 ` Kirill Tkhai
2014-11-11 13:07 ` Wanpeng Li
0 siblings, 1 reply; 6+ messages in thread
From: Kirill Tkhai @ 2014-11-11 11:10 UTC (permalink / raw)
To: Wanpeng Li; +Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, linux-kernel
В Вт, 11/11/2014 в 10:30 +0800, Wanpeng Li пишет:
> I observe that dl task can't be migrated to other cpus during cpu hotplug, in
> addition, task may/may not be running again if cpu is added back. The root cause
> which I found is that dl task will be throtted and removed from dl rq after
> comsuming all budget, which leads to stop task can't pick it up from dl rq and
> migrate to other cpus during hotplug.
>
> The method to reproduce:
> schedtool -E -t 50000:100000 -e ./test
> Actually test is just a simple for loop. Then observe which cpu the test
> task is on.
> echo 0 > /sys/devices/system/cpu/cpuN/online
>
> This patch fix it by push the task to another cpu in dl_task_timer() if
> rq is offline.
>
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
I'm still thinking we don't have to guarantee any "deadlines" during cpu hotplug...
But, if speaking about this way:
> ---
> v3 -> v4:
> * use tsk_cpus_allowed wrapper
> * fix compile error
> v2 -> v3:
> * don't get_task_struct
> * if cannot preempt any rq, fallback to pick any online cpus
> * use cpu_active_mask as original later_mask if cpu is offline
> v1 -> v2:
> * push the task to another cpu in dl_task_timer() if rq is offline.
>
>
> kernel/sched/deadline.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 00324af..e0fbba4 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -487,6 +487,7 @@ static int start_dl_timer(struct sched_dl_entity *dl_se, bool boosted)
> return hrtimer_active(&dl_se->dl_timer);
> }
>
> +static struct rq *find_lock_later_rq(struct task_struct *task, struct rq *rq);
> /*
> * This is the bandwidth enforcement timer callback. If here, we know
> * a task is not on its dl_rq, since the fact that the timer was running
> @@ -538,6 +539,46 @@ again:
> update_rq_clock(rq);
> dl_se->dl_throttled = 0;
> dl_se->dl_yielded = 0;
> +
> + /*
> + * So if we find that the rq the task was on is no longer
> + * available, we need to select a new rq.
> + */
> + if (!rq->online) {
> + struct rq *later_rq = NULL;
> +
> + raw_spin_unlock(&rq->lock);
> +
> + later_rq = find_lock_later_rq(p, rq);
find_lock_later_rq() expects that rq is locked.
The comment near its head confuses a reader. It locks newly found rq.
> +
> + if (!later_rq) {
> + int cpu;
> +
> + /*
> + * If cannot preempt any rq, fallback to pick any
> + * online cpu.
> + */
> + for_each_cpu(cpu, tsk_cpus_allowed(p))
> + if (cpu_online(cpu))
> + later_rq = cpu_rq(cpu);
> + if (!later_rq) {
> + pr_warn("fail to find any online and task "
> + "will never come back to us\n");
> + goto out;
> + }
> + }
> +
> + deactivate_task(rq, p, 0);
> + set_task_cpu(p, later_rq->cpu);
> + activate_task(later_rq, p, 0);
> +
> + resched_curr(later_rq);
> +
> + double_unlock_balance(rq, later_rq);
double_unlock_balance() unlocks later_rq only.
> +
> + goto out;
> + }
> +
> if (task_on_rq_queued(p)) {
> enqueue_task_dl(rq, p, ENQUEUE_REPLENISH);
> if (dl_task(rq->curr))
> @@ -555,7 +596,7 @@ again:
> }
> unlock:
> raw_spin_unlock(&rq->lock);
> -
> +out:
> return HRTIMER_NORESTART;
> }
>
> @@ -1185,8 +1226,12 @@ static int find_later_rq(struct task_struct *task)
> * We have to consider system topology and task affinity
> * first, then we can look for a suitable cpu.
> */
> - cpumask_copy(later_mask, task_rq(task)->rd->span);
> - cpumask_and(later_mask, later_mask, cpu_active_mask);
> + if (likely(task_rq(task)->online)) {
> + cpumask_copy(later_mask, task_rq(task)->rd->span);
> + cpumask_and(later_mask, later_mask, cpu_active_mask);
> + } else
> + /* for offline cpus we have a singleton rd */
> + cpumask_copy(later_mask, cpu_active_mask);
> cpumask_and(later_mask, later_mask, &task->cpus_allowed);
> best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
> task, later_mask);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] sched/deadline: support dl task migration during cpu hotplug
2014-11-11 11:10 ` Kirill Tkhai
@ 2014-11-11 13:07 ` Wanpeng Li
2014-11-11 13:09 ` Kirill Tkhai
0 siblings, 1 reply; 6+ messages in thread
From: Wanpeng Li @ 2014-11-11 13:07 UTC (permalink / raw)
To: Kirill Tkhai, Wanpeng Li
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, linux-kernel
Hi Kirill,
On 11/11/14, 7:10 PM, Kirill Tkhai wrote:
> В Вт, 11/11/2014 в 10:30 +0800, Wanpeng Li пишет:
>> I observe that dl task can't be migrated to other cpus during cpu hotplug, in
>> addition, task may/may not be running again if cpu is added back. The root cause
>> which I found is that dl task will be throtted and removed from dl rq after
>> comsuming all budget, which leads to stop task can't pick it up from dl rq and
>> migrate to other cpus during hotplug.
>>
>> The method to reproduce:
>> schedtool -E -t 50000:100000 -e ./test
>> Actually test is just a simple for loop. Then observe which cpu the test
>> task is on.
>> echo 0 > /sys/devices/system/cpu/cpuN/online
>>
>> This patch fix it by push the task to another cpu in dl_task_timer() if
>> rq is offline.
>>
>> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> I'm still thinking we don't have to guarantee any "deadlines" during cpu hotplug...
> But, if speaking about this way:
>
>> ---
>> v3 -> v4:
>> * use tsk_cpus_allowed wrapper
>> * fix compile error
>> v2 -> v3:
>> * don't get_task_struct
>> * if cannot preempt any rq, fallback to pick any online cpus
>> * use cpu_active_mask as original later_mask if cpu is offline
>> v1 -> v2:
>> * push the task to another cpu in dl_task_timer() if rq is offline.
>>
>>
>> kernel/sched/deadline.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 48 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
>> index 00324af..e0fbba4 100644
>> --- a/kernel/sched/deadline.c
>> +++ b/kernel/sched/deadline.c
>> @@ -487,6 +487,7 @@ static int start_dl_timer(struct sched_dl_entity *dl_se, bool boosted)
>> return hrtimer_active(&dl_se->dl_timer);
>> }
>>
>> +static struct rq *find_lock_later_rq(struct task_struct *task, struct rq *rq);
>> /*
>> * This is the bandwidth enforcement timer callback. If here, we know
>> * a task is not on its dl_rq, since the fact that the timer was running
>> @@ -538,6 +539,46 @@ again:
>> update_rq_clock(rq);
>> dl_se->dl_throttled = 0;
>> dl_se->dl_yielded = 0;
>> +
>> + /*
>> + * So if we find that the rq the task was on is no longer
>> + * available, we need to select a new rq.
>> + */
>> + if (!rq->online) {
>> + struct rq *later_rq = NULL;
>> +
>> + raw_spin_unlock(&rq->lock);
>> +
>> + later_rq = find_lock_later_rq(p, rq);
> find_lock_later_rq() expects that rq is locked.
>
> The comment near its head confuses a reader. It locks newly found rq.
Sorry for my bad, what's you think should be changed?
Regards,
Wanpeng Li
>
>> +
>> + if (!later_rq) {
>> + int cpu;
>> +
>> + /*
>> + * If cannot preempt any rq, fallback to pick any
>> + * online cpu.
>> + */
>> + for_each_cpu(cpu, tsk_cpus_allowed(p))
>> + if (cpu_online(cpu))
>> + later_rq = cpu_rq(cpu);
>> + if (!later_rq) {
>> + pr_warn("fail to find any online and task "
>> + "will never come back to us\n");
>> + goto out;
>> + }
>> + }
>> +
>> + deactivate_task(rq, p, 0);
>> + set_task_cpu(p, later_rq->cpu);
>> + activate_task(later_rq, p, 0);
>> +
>> + resched_curr(later_rq);
>> +
>> + double_unlock_balance(rq, later_rq);
> double_unlock_balance() unlocks later_rq only.
>
>> +
>> + goto out;
>> + }
>> +
>> if (task_on_rq_queued(p)) {
>> enqueue_task_dl(rq, p, ENQUEUE_REPLENISH);
>> if (dl_task(rq->curr))
>> @@ -555,7 +596,7 @@ again:
>> }
>> unlock:
>> raw_spin_unlock(&rq->lock);
>> -
>> +out:
>> return HRTIMER_NORESTART;
>> }
>>
>> @@ -1185,8 +1226,12 @@ static int find_later_rq(struct task_struct *task)
>> * We have to consider system topology and task affinity
>> * first, then we can look for a suitable cpu.
>> */
>> - cpumask_copy(later_mask, task_rq(task)->rd->span);
>> - cpumask_and(later_mask, later_mask, cpu_active_mask);
>> + if (likely(task_rq(task)->online)) {
>> + cpumask_copy(later_mask, task_rq(task)->rd->span);
>> + cpumask_and(later_mask, later_mask, cpu_active_mask);
>> + } else
>> + /* for offline cpus we have a singleton rd */
>> + cpumask_copy(later_mask, cpu_active_mask);
>> cpumask_and(later_mask, later_mask, &task->cpus_allowed);
>> best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
>> task, later_mask);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] sched/deadline: support dl task migration during cpu hotplug
2014-11-11 13:07 ` Wanpeng Li
@ 2014-11-11 13:09 ` Kirill Tkhai
2014-11-11 23:53 ` Wanpeng Li
0 siblings, 1 reply; 6+ messages in thread
From: Kirill Tkhai @ 2014-11-11 13:09 UTC (permalink / raw)
To: Wanpeng Li
Cc: Wanpeng Li, Ingo Molnar, Peter Zijlstra, Juri Lelli, linux-kernel
В Вт, 11/11/2014 в 21:07 +0800, Wanpeng Li пишет:
> Hi Kirill,
> On 11/11/14, 7:10 PM, Kirill Tkhai wrote:
> > В Вт, 11/11/2014 в 10:30 +0800, Wanpeng Li пишет:
> >> I observe that dl task can't be migrated to other cpus during cpu hotplug, in
> >> addition, task may/may not be running again if cpu is added back. The root cause
> >> which I found is that dl task will be throtted and removed from dl rq after
> >> comsuming all budget, which leads to stop task can't pick it up from dl rq and
> >> migrate to other cpus during hotplug.
> >>
> >> The method to reproduce:
> >> schedtool -E -t 50000:100000 -e ./test
> >> Actually test is just a simple for loop. Then observe which cpu the test
> >> task is on.
> >> echo 0 > /sys/devices/system/cpu/cpuN/online
> >>
> >> This patch fix it by push the task to another cpu in dl_task_timer() if
> >> rq is offline.
> >>
> >> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> > I'm still thinking we don't have to guarantee any "deadlines" during cpu hotplug...
> > But, if speaking about this way:
> >
> >> ---
> >> v3 -> v4:
> >> * use tsk_cpus_allowed wrapper
> >> * fix compile error
> >> v2 -> v3:
> >> * don't get_task_struct
> >> * if cannot preempt any rq, fallback to pick any online cpus
> >> * use cpu_active_mask as original later_mask if cpu is offline
> >> v1 -> v2:
> >> * push the task to another cpu in dl_task_timer() if rq is offline.
> >>
> >>
> >> kernel/sched/deadline.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
> >> 1 file changed, 48 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> >> index 00324af..e0fbba4 100644
> >> --- a/kernel/sched/deadline.c
> >> +++ b/kernel/sched/deadline.c
> >> @@ -487,6 +487,7 @@ static int start_dl_timer(struct sched_dl_entity *dl_se, bool boosted)
> >> return hrtimer_active(&dl_se->dl_timer);
> >> }
> >>
> >> +static struct rq *find_lock_later_rq(struct task_struct *task, struct rq *rq);
> >> /*
> >> * This is the bandwidth enforcement timer callback. If here, we know
> >> * a task is not on its dl_rq, since the fact that the timer was running
> >> @@ -538,6 +539,46 @@ again:
> >> update_rq_clock(rq);
> >> dl_se->dl_throttled = 0;
> >> dl_se->dl_yielded = 0;
> >> +
> >> + /*
> >> + * So if we find that the rq the task was on is no longer
> >> + * available, we need to select a new rq.
> >> + */
> >> + if (!rq->online) {
> >> + struct rq *later_rq = NULL;
> >> +
> >> + raw_spin_unlock(&rq->lock);
> >> +
> >> + later_rq = find_lock_later_rq(p, rq);
> > find_lock_later_rq() expects that rq is locked.
> >
> > The comment near its head confuses a reader. It locks newly found rq.
>
> Sorry for my bad, what's you think should be changed?
raw_spin_unlock(&rq->lock) is wrong here. It's not need.
>
> >
> >> +
> >> + if (!later_rq) {
> >> + int cpu;
> >> +
> >> + /*
> >> + * If cannot preempt any rq, fallback to pick any
> >> + * online cpu.
> >> + */
> >> + for_each_cpu(cpu, tsk_cpus_allowed(p))
> >> + if (cpu_online(cpu))
> >> + later_rq = cpu_rq(cpu);
> >> + if (!later_rq) {
> >> + pr_warn("fail to find any online and task "
> >> + "will never come back to us\n");
> >> + goto out;
> >> + }
> >> + }
> >> +
> >> + deactivate_task(rq, p, 0);
> >> + set_task_cpu(p, later_rq->cpu);
> >> + activate_task(later_rq, p, 0);
> >> +
> >> + resched_curr(later_rq);
> >> +
> >> + double_unlock_balance(rq, later_rq);
> > double_unlock_balance() unlocks later_rq only.
> >
> >> +
> >> + goto out;
> >> + }
> >> +
> >> if (task_on_rq_queued(p)) {
> >> enqueue_task_dl(rq, p, ENQUEUE_REPLENISH);
> >> if (dl_task(rq->curr))
> >> @@ -555,7 +596,7 @@ again:
> >> }
> >> unlock:
> >> raw_spin_unlock(&rq->lock);
> >> -
> >> +out:
> >> return HRTIMER_NORESTART;
> >> }
> >>
> >> @@ -1185,8 +1226,12 @@ static int find_later_rq(struct task_struct *task)
> >> * We have to consider system topology and task affinity
> >> * first, then we can look for a suitable cpu.
> >> */
> >> - cpumask_copy(later_mask, task_rq(task)->rd->span);
> >> - cpumask_and(later_mask, later_mask, cpu_active_mask);
> >> + if (likely(task_rq(task)->online)) {
> >> + cpumask_copy(later_mask, task_rq(task)->rd->span);
> >> + cpumask_and(later_mask, later_mask, cpu_active_mask);
> >> + } else
> >> + /* for offline cpus we have a singleton rd */
> >> + cpumask_copy(later_mask, cpu_active_mask);
> >> cpumask_and(later_mask, later_mask, &task->cpus_allowed);
> >> best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
> >> task, later_mask);
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] sched/deadline: support dl task migration during cpu hotplug
2014-11-11 13:09 ` Kirill Tkhai
@ 2014-11-11 23:53 ` Wanpeng Li
2014-11-12 0:12 ` Wanpeng Li
0 siblings, 1 reply; 6+ messages in thread
From: Wanpeng Li @ 2014-11-11 23:53 UTC (permalink / raw)
To: Kirill Tkhai
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, linux-kernel, Wanpeng Li
Hi Kirill,
On Tue, Nov 11, 2014 at 04:09:14PM +0300, Kirill Tkhai wrote:
>В Вт, 11/11/2014 в 21:07 +0800, Wanpeng Li пишет:
>> Hi Kirill,
>> On 11/11/14, 7:10 PM, Kirill Tkhai wrote:
>> > В Вт, 11/11/2014 в 10:30 +0800, Wanpeng Li пишет:
>> >> I observe that dl task can't be migrated to other cpus during cpu hotplug, in
>> >> addition, task may/may not be running again if cpu is added back. The root cause
>> >> which I found is that dl task will be throtted and removed from dl rq after
>> >> comsuming all budget, which leads to stop task can't pick it up from dl rq and
>> >> migrate to other cpus during hotplug.
>> >>
>> >> The method to reproduce:
>> >> schedtool -E -t 50000:100000 -e ./test
>> >> Actually test is just a simple for loop. Then observe which cpu the test
>> >> task is on.
>> >> echo 0 > /sys/devices/system/cpu/cpuN/online
>> >>
>> >> This patch fix it by push the task to another cpu in dl_task_timer() if
>> >> rq is offline.
>> >>
>> >> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>> > I'm still thinking we don't have to guarantee any "deadlines" during cpu hotplug...
>> > But, if speaking about this way:
>> >
>> >> ---
>> >> v3 -> v4:
>> >> * use tsk_cpus_allowed wrapper
>> >> * fix compile error
>> >> v2 -> v3:
>> >> * don't get_task_struct
>> >> * if cannot preempt any rq, fallback to pick any online cpus
>> >> * use cpu_active_mask as original later_mask if cpu is offline
>> >> v1 -> v2:
>> >> * push the task to another cpu in dl_task_timer() if rq is offline.
>> >>
>> >>
>> >> kernel/sched/deadline.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
>> >> 1 file changed, 48 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
>> >> index 00324af..e0fbba4 100644
>> >> --- a/kernel/sched/deadline.c
>> >> +++ b/kernel/sched/deadline.c
>> >> @@ -487,6 +487,7 @@ static int start_dl_timer(struct sched_dl_entity *dl_se, bool boosted)
>> >> return hrtimer_active(&dl_se->dl_timer);
>> >> }
>> >>
>> >> +static struct rq *find_lock_later_rq(struct task_struct *task, struct rq *rq);
>> >> /*
>> >> * This is the bandwidth enforcement timer callback. If here, we know
>> >> * a task is not on its dl_rq, since the fact that the timer was running
>> >> @@ -538,6 +539,46 @@ again:
>> >> update_rq_clock(rq);
>> >> dl_se->dl_throttled = 0;
>> >> dl_se->dl_yielded = 0;
>> >> +
>> >> + /*
>> >> + * So if we find that the rq the task was on is no longer
>> >> + * available, we need to select a new rq.
>> >> + */
>> >> + if (!rq->online) {
>> >> + struct rq *later_rq = NULL;
>> >> +
>> >> + raw_spin_unlock(&rq->lock);
>> >> +
>> >> + later_rq = find_lock_later_rq(p, rq);
>> > find_lock_later_rq() expects that rq is locked.
>> >
>> > The comment near its head confuses a reader. It locks newly found rq.
>>
>> Sorry for my bad, what's you think should be changed?
>
>raw_spin_unlock(&rq->lock) is wrong here. It's not need.
>
The machine down after remove this.
Regards,
Wanpeng Li
>>
>> >
>> >> +
>> >> + if (!later_rq) {
>> >> + int cpu;
>> >> +
>> >> + /*
>> >> + * If cannot preempt any rq, fallback to pick any
>> >> + * online cpu.
>> >> + */
>> >> + for_each_cpu(cpu, tsk_cpus_allowed(p))
>> >> + if (cpu_online(cpu))
>> >> + later_rq = cpu_rq(cpu);
>> >> + if (!later_rq) {
>> >> + pr_warn("fail to find any online and task "
>> >> + "will never come back to us\n");
>> >> + goto out;
>> >> + }
>> >> + }
>> >> +
>> >> + deactivate_task(rq, p, 0);
>> >> + set_task_cpu(p, later_rq->cpu);
>> >> + activate_task(later_rq, p, 0);
>> >> +
>> >> + resched_curr(later_rq);
>> >> +
>> >> + double_unlock_balance(rq, later_rq);
>> > double_unlock_balance() unlocks later_rq only.
>> >
>> >> +
>> >> + goto out;
>> >> + }
>> >> +
>> >> if (task_on_rq_queued(p)) {
>> >> enqueue_task_dl(rq, p, ENQUEUE_REPLENISH);
>> >> if (dl_task(rq->curr))
>> >> @@ -555,7 +596,7 @@ again:
>> >> }
>> >> unlock:
>> >> raw_spin_unlock(&rq->lock);
>> >> -
>> >> +out:
>> >> return HRTIMER_NORESTART;
>> >> }
>> >>
>> >> @@ -1185,8 +1226,12 @@ static int find_later_rq(struct task_struct *task)
>> >> * We have to consider system topology and task affinity
>> >> * first, then we can look for a suitable cpu.
>> >> */
>> >> - cpumask_copy(later_mask, task_rq(task)->rd->span);
>> >> - cpumask_and(later_mask, later_mask, cpu_active_mask);
>> >> + if (likely(task_rq(task)->online)) {
>> >> + cpumask_copy(later_mask, task_rq(task)->rd->span);
>> >> + cpumask_and(later_mask, later_mask, cpu_active_mask);
>> >> + } else
>> >> + /* for offline cpus we have a singleton rd */
>> >> + cpumask_copy(later_mask, cpu_active_mask);
>> >> cpumask_and(later_mask, later_mask, &task->cpus_allowed);
>> >> best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
>> >> task, later_mask);
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> > Please read the FAQ at http://www.tux.org/lkml/
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] sched/deadline: support dl task migration during cpu hotplug
2014-11-11 23:53 ` Wanpeng Li
@ 2014-11-12 0:12 ` Wanpeng Li
0 siblings, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2014-11-12 0:12 UTC (permalink / raw)
To: Kirill Tkhai
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, linux-kernel, Wanpeng Li
On Wed, Nov 12, 2014 at 07:53:20AM +0800, Wanpeng Li wrote:
>Hi Kirill,
>On Tue, Nov 11, 2014 at 04:09:14PM +0300, Kirill Tkhai wrote:
>>В Вт, 11/11/2014 в 21:07 +0800, Wanpeng Li пишет:
>>> Hi Kirill,
>>> On 11/11/14, 7:10 PM, Kirill Tkhai wrote:
>>> > В Вт, 11/11/2014 в 10:30 +0800, Wanpeng Li пишет:
>>> >> I observe that dl task can't be migrated to other cpus during cpu hotplug, in
>>> >> addition, task may/may not be running again if cpu is added back. The root cause
>>> >> which I found is that dl task will be throtted and removed from dl rq after
>>> >> comsuming all budget, which leads to stop task can't pick it up from dl rq and
>>> >> migrate to other cpus during hotplug.
>>> >>
>>> >> The method to reproduce:
>>> >> schedtool -E -t 50000:100000 -e ./test
>>> >> Actually test is just a simple for loop. Then observe which cpu the test
>>> >> task is on.
>>> >> echo 0 > /sys/devices/system/cpu/cpuN/online
>>> >>
>>> >> This patch fix it by push the task to another cpu in dl_task_timer() if
>>> >> rq is offline.
>>> >>
>>> >> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>>> > I'm still thinking we don't have to guarantee any "deadlines" during cpu hotplug...
>>> > But, if speaking about this way:
>>> >
>>> >> ---
>>> >> v3 -> v4:
>>> >> * use tsk_cpus_allowed wrapper
>>> >> * fix compile error
>>> >> v2 -> v3:
>>> >> * don't get_task_struct
>>> >> * if cannot preempt any rq, fallback to pick any online cpus
>>> >> * use cpu_active_mask as original later_mask if cpu is offline
>>> >> v1 -> v2:
>>> >> * push the task to another cpu in dl_task_timer() if rq is offline.
>>> >>
>>> >>
>>> >> kernel/sched/deadline.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
>>> >> 1 file changed, 48 insertions(+), 3 deletions(-)
>>> >>
>>> >> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
>>> >> index 00324af..e0fbba4 100644
>>> >> --- a/kernel/sched/deadline.c
>>> >> +++ b/kernel/sched/deadline.c
>>> >> @@ -487,6 +487,7 @@ static int start_dl_timer(struct sched_dl_entity *dl_se, bool boosted)
>>> >> return hrtimer_active(&dl_se->dl_timer);
>>> >> }
>>> >>
>>> >> +static struct rq *find_lock_later_rq(struct task_struct *task, struct rq *rq);
>>> >> /*
>>> >> * This is the bandwidth enforcement timer callback. If here, we know
>>> >> * a task is not on its dl_rq, since the fact that the timer was running
>>> >> @@ -538,6 +539,46 @@ again:
>>> >> update_rq_clock(rq);
>>> >> dl_se->dl_throttled = 0;
>>> >> dl_se->dl_yielded = 0;
>>> >> +
>>> >> + /*
>>> >> + * So if we find that the rq the task was on is no longer
>>> >> + * available, we need to select a new rq.
>>> >> + */
>>> >> + if (!rq->online) {
>>> >> + struct rq *later_rq = NULL;
>>> >> +
>>> >> + raw_spin_unlock(&rq->lock);
>>> >> +
>>> >> + later_rq = find_lock_later_rq(p, rq);
>>> > find_lock_later_rq() expects that rq is locked.
>>> >
>>> > The comment near its head confuses a reader. It locks newly found rq.
>>>
>>> Sorry for my bad, what's you think should be changed?
>>
>>raw_spin_unlock(&rq->lock) is wrong here. It's not need.
>>
>
>The machine down after remove this.
Just solve it, sorry for the noise.
Regards,
Wanpeng Li
>
>Regards,
>Wanpeng Li
>
>>>
>>> >
>>> >> +
>>> >> + if (!later_rq) {
>>> >> + int cpu;
>>> >> +
>>> >> + /*
>>> >> + * If cannot preempt any rq, fallback to pick any
>>> >> + * online cpu.
>>> >> + */
>>> >> + for_each_cpu(cpu, tsk_cpus_allowed(p))
>>> >> + if (cpu_online(cpu))
>>> >> + later_rq = cpu_rq(cpu);
>>> >> + if (!later_rq) {
>>> >> + pr_warn("fail to find any online and task "
>>> >> + "will never come back to us\n");
>>> >> + goto out;
>>> >> + }
>>> >> + }
>>> >> +
>>> >> + deactivate_task(rq, p, 0);
>>> >> + set_task_cpu(p, later_rq->cpu);
>>> >> + activate_task(later_rq, p, 0);
>>> >> +
>>> >> + resched_curr(later_rq);
>>> >> +
>>> >> + double_unlock_balance(rq, later_rq);
>>> > double_unlock_balance() unlocks later_rq only.
>>> >
>>> >> +
>>> >> + goto out;
>>> >> + }
>>> >> +
>>> >> if (task_on_rq_queued(p)) {
>>> >> enqueue_task_dl(rq, p, ENQUEUE_REPLENISH);
>>> >> if (dl_task(rq->curr))
>>> >> @@ -555,7 +596,7 @@ again:
>>> >> }
>>> >> unlock:
>>> >> raw_spin_unlock(&rq->lock);
>>> >> -
>>> >> +out:
>>> >> return HRTIMER_NORESTART;
>>> >> }
>>> >>
>>> >> @@ -1185,8 +1226,12 @@ static int find_later_rq(struct task_struct *task)
>>> >> * We have to consider system topology and task affinity
>>> >> * first, then we can look for a suitable cpu.
>>> >> */
>>> >> - cpumask_copy(later_mask, task_rq(task)->rd->span);
>>> >> - cpumask_and(later_mask, later_mask, cpu_active_mask);
>>> >> + if (likely(task_rq(task)->online)) {
>>> >> + cpumask_copy(later_mask, task_rq(task)->rd->span);
>>> >> + cpumask_and(later_mask, later_mask, cpu_active_mask);
>>> >> + } else
>>> >> + /* for offline cpus we have a singleton rd */
>>> >> + cpumask_copy(later_mask, cpu_active_mask);
>>> >> cpumask_and(later_mask, later_mask, &task->cpus_allowed);
>>> >> best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
>>> >> task, later_mask);
>>> >
>>> > --
>>> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> > the body of a message to majordomo@vger.kernel.org
>>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> > Please read the FAQ at http://www.tux.org/lkml/
>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-11-12 0:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-11 2:30 [PATCH v4] sched/deadline: support dl task migration during cpu hotplug Wanpeng Li
2014-11-11 11:10 ` Kirill Tkhai
2014-11-11 13:07 ` Wanpeng Li
2014-11-11 13:09 ` Kirill Tkhai
2014-11-11 23:53 ` Wanpeng Li
2014-11-12 0:12 ` Wanpeng Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox