Messages in this thread | | | Subject | Re: [PATCH] sched/deadline: fix dl bandwidth of root domain overflow after dl task dead | From | Wanpeng Li <> | Date | Sun, 30 Aug 2015 19:25:08 +0800 |
| |
On 8/10/15 10:10 PM, Juri Lelli wrote: > On 06/08/15 09:39, Wanpeng Li wrote: >> Hi Juri, >> > Hi, > >> 2015-05-06 16:14 GMT+08:00 Juri Lelli <juri.lelli@arm.com >> <mailto:juri.lelli@arm.com>>: >> >> Hi Wanpeng, >> >> I finally got to review this, sorry about the huge delay. >> >> On 07/04/2015 04:36, Wanpeng Li wrote: >> > The total used dl bandwidth of each root domain will be reset to 0 after >> > cpu hotplug when rebuild sched domains, since the call path is: >> > >> > _cpu_down >> > cpuset_cpu_inactive() >> > cpuset_update_active_cpus() >> > partition_sched_domains() >> > build_sched_domains() >> > init_rootdomain() >> > init_dl_bw() >> > >> > The bandwidth which dl task occupy will be released when dl task dead, >> > it will be minus from total used dl bandwidth of its root domain, >> > however, bandwidth overflow occurs since total used dl bandwidth is 0. >> > >> >> Right, that's a bug. >> >> > This patch fix it by attaching the bandwidth which dl task occupy to >> > the new root domain when the task is migrating since cpu hotplug, and >> > attach all the used dl bandwidth of dl tasks to the new root domain >> > when sched domains are rebuild. >> > >> >> But, I think this fix has still a couple of problems: >> >> - what happens if a DL task is simply sleeping when domains are >> reconfigured? >> >> - def_root_domain has now multiple accounting problems, as you do >> this thing even when a cpu is moved there in the cpuoff path >> >> Also, runqueue (and throttling) information are dynamic, while we >> are trying to fix a static problem. It's probably not a good idea >> mixing them. >> >> I'm not sure how (I need more time to think it through), but can >> we maybe fix this using cpuset information? >> >> >> Any ideas? >> > Yes, actually. I might have a different fix, but I'd like to play with > it a bit more as it is a bit too intrusive. Let me see if I can come > up with something that I can share.
Ping Peter, Juri, any detail ideas to help me post another version of my patch? ;-)
Regards, Wanpeng Li
> > Thanks, > > - Juri > >> Regards, >> Wanpeng Li >> >> >> >> Thanks, >> >> - Juri >> >> > Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com >> <mailto:wanpeng.li@linux.intel.com>> >> > --- >> > kernel/sched/core.c | 1 + >> > kernel/sched/deadline.c | 25 +++++++++++++++++++++++++ >> > kernel/sched/sched.h | 1 + >> > 3 files changed, 27 insertions(+) >> > >> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> > index 28b0d75..c940999 100644 >> > --- a/kernel/sched/core.c >> > +++ b/kernel/sched/core.c >> > @@ -5586,6 +5586,7 @@ static void rq_attach_root(struct rq *rq, >> struct root_domain *rd) >> > rq->rd = rd; >> > >> > cpumask_set_cpu(rq->cpu, rd->span); >> > + attach_dl_bw(rq); >> > if (cpumask_test_cpu(rq->cpu, cpu_active_mask)) >> > set_rq_online(rq); >> > >> > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c >> > index 5e95145..62680d7 100644 >> > --- a/kernel/sched/deadline.c >> > +++ b/kernel/sched/deadline.c >> > @@ -224,6 +224,7 @@ static void dl_task_offline_migration(struct >> rq *rq, struct task_struct *p) >> > { >> > struct rq *later_rq = NULL; >> > bool fallback = false; >> > + struct dl_bw *dl_b; >> > >> > later_rq = find_lock_later_rq(p, rq); >> > >> > @@ -258,6 +259,11 @@ static void dl_task_offline_migration(struct >> rq *rq, struct task_struct *p) >> > set_task_cpu(p, later_rq->cpu); >> > activate_task(later_rq, p, ENQUEUE_REPLENISH); >> > >> > + dl_b = dl_bw_of(later_rq->cpu); >> > + raw_spin_lock(&dl_b->lock); >> > + __dl_add(dl_b, p->dl.dl_bw); >> > + raw_spin_unlock(&dl_b->lock); >> > + >> > if (!fallback) >> > resched_curr(later_rq); >> > >> > @@ -1776,6 +1782,25 @@ static void prio_changed_dl(struct rq *rq, >> struct task_struct *p, >> > switched_to_dl(rq, p); >> > } >> > >> > +void attach_dl_bw(struct rq *rq) >> > +{ >> > + struct rb_node *next_node = rq->dl.rb_leftmost; >> > + struct sched_dl_entity *dl_se; >> > + struct dl_bw *dl_b; >> > + >> > + dl_b = dl_bw_of(rq->cpu); >> > + raw_spin_lock(&dl_b->lock); >> > +next_node: >> > + if (next_node) { >> > + dl_se = rb_entry(next_node, struct sched_dl_entity, >> rb_node); >> > + __dl_add(dl_b, dl_se->dl_bw); >> > + next_node = rb_next(next_node); >> > + >> > + goto next_node; >> > + } >> > + raw_spin_unlock(&dl_b->lock); >> > +} >> > + >> > const struct sched_class dl_sched_class = { >> > .next = &rt_sched_class, >> > .enqueue_task = enqueue_task_dl, >> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h >> > index e0e1299..a7b1a59 100644 >> > --- a/kernel/sched/sched.h >> > +++ b/kernel/sched/sched.h >> > @@ -1676,6 +1676,7 @@ extern void init_dl_rq(struct dl_rq *dl_rq); >> > >> > extern void cfs_bandwidth_usage_inc(void); >> > extern void cfs_bandwidth_usage_dec(void); >> > +void attach_dl_bw(struct rq *rq); >> > >> > #ifdef CONFIG_NO_HZ_COMMON >> > enum rq_nohz_flag_bits { >> > >> >> -- >> To unsubscribe from this list: send the line "unsubscribe >> linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> <mailto: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/ >>
| |