Messages in this thread | | | Date | Tue, 01 Sep 2015 10:49:37 +0100 | From | Juri Lelli <> | Subject | Re: [PATCH] sched/deadline: fix dl bandwidth of root domain overflow after dl task dead |
| |
Hi,
On 30/08/15 12:25, Wanpeng Li wrote: > 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? ;-) >
Let me see if I'm able to post my version of the fix before end of this week ;).
Thanks!
- Juri
> 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/ >>> >
| |