Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932549AbcDAPTe (ORCPT ); Fri, 1 Apr 2016 11:19:34 -0400 Received: from mail-wm0-f42.google.com ([74.125.82.42]:33276 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932164AbcDAPSZ (ORCPT ); Fri, 1 Apr 2016 11:18:25 -0400 From: Luca Abeni To: linux-kernel@vger.kernel.org Cc: Peter Zijlstra , Ingo Molnar , Juri Lelli , Luca Abeni Subject: [RFC v2 4/7] Fix the update of the total -deadline utilization Date: Fri, 1 Apr 2016 17:12:30 +0200 Message-Id: <1459523553-29089-5-git-send-email-luca.abeni@unitn.it> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1459523553-29089-1-git-send-email-luca.abeni@unitn.it> References: <1459523553-29089-1-git-send-email-luca.abeni@unitn.it> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4753 Lines: 141 Now that the inactive timer can be armed to fire at the 0-lag time, it is possible to use inactive_task_timer() to update the total -deadline utilization (dl_b->total_bw) at the correct time, fixing dl_overflow() and __setparam_dl(). Signed-off-by: Luca Abeni --- kernel/sched/core.c | 36 ++++++++++++------------------------ kernel/sched/deadline.c | 33 +++++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 23d235c..4158d1f 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2347,9 +2347,6 @@ static inline int dl_bw_cpus(int i) * allocated bandwidth to reflect the new situation. * * This function is called while holding p's rq->lock. - * - * XXX we should delay bw change until the task's 0-lag point, see - * __setparam_dl(). */ static int dl_overflow(struct task_struct *p, int policy, const struct sched_attr *attr) @@ -2377,11 +2374,22 @@ static int dl_overflow(struct task_struct *p, int policy, err = 0; } else if (dl_policy(policy) && task_has_dl_policy(p) && !__dl_overflow(dl_b, cpus, p->dl.dl_bw, new_bw)) { + /* + * XXX this is slightly incorrect: when the task + * utilization decreases, we should delay the total + * utilization change until the task's 0-lag point. + * But this would require to set the task's "inactive + * timer" when the task is not inactive. + */ __dl_clear(dl_b, p->dl.dl_bw); __dl_add(dl_b, new_bw); err = 0; } else if (!dl_policy(policy) && task_has_dl_policy(p)) { - __dl_clear(dl_b, p->dl.dl_bw); + /* + * Do not decrease the total deadline utilization here, + * switched_from_dl() will take care to do it at the correct + * (0-lag) time. + */ err = 0; } raw_spin_unlock(&dl_b->lock); @@ -3647,26 +3655,6 @@ __setparam_dl(struct task_struct *p, const struct sched_attr *attr) dl_se->dl_period = attr->sched_period ?: dl_se->dl_deadline; dl_se->flags = attr->sched_flags; dl_se->dl_bw = to_ratio(dl_se->dl_period, dl_se->dl_runtime); - - /* - * Changing the parameters of a task is 'tricky' and we're not doing - * the correct thing -- also see task_dead_dl() and switched_from_dl(). - * - * What we SHOULD do is delay the bandwidth release until the 0-lag - * point. This would include retaining the task_struct until that time - * and change dl_overflow() to not immediately decrement the current - * amount. - * - * Instead we retain the current runtime/deadline and let the new - * parameters take effect after the current reservation period lapses. - * This is safe (albeit pessimistic) because the 0-lag point is always - * before the current scheduling deadline. - * - * We can still have temporary overloads because we do not delay the - * change in bandwidth until that time; so admission control is - * not on the safe side. It does however guarantee tasks will never - * consume more than promised. - */ } /* diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 97cd5f2..ca7910a 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -98,8 +98,14 @@ static void task_go_inactive(struct task_struct *p) */ if (ktime_us_delta(act, now) < 0) { sub_running_bw(dl_se, dl_rq); - if (!dl_task(p)) + if (!dl_task(p)) { + struct dl_bw *dl_b = dl_bw_of(task_cpu(p)); + + raw_spin_lock(&dl_b->lock); + __dl_clear(dl_b, p->dl.dl_bw); __dl_clear_params(p); + raw_spin_unlock(&dl_b->lock); + } return; } @@ -865,8 +871,13 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer) rq = task_rq_lock(p, &flags); - if (!dl_task(p)) { + if (!dl_task(p) || p->state == TASK_DEAD) { + struct dl_bw *dl_b = dl_bw_of(task_cpu(p)); + + raw_spin_lock(&dl_b->lock); + __dl_clear(dl_b, p->dl.dl_bw); __dl_clear_params(p); + raw_spin_unlock(&dl_b->lock); goto unlock; } @@ -1341,15 +1352,21 @@ static void task_fork_dl(struct task_struct *p) static void task_dead_dl(struct task_struct *p) { - struct dl_bw *dl_b = dl_bw_of(task_cpu(p)); - /* * Since we are TASK_DEAD we won't slip out of the domain! */ - raw_spin_lock_irq(&dl_b->lock); - /* XXX we should retain the bw until 0-lag */ - dl_b->total_bw -= p->dl.dl_bw; - raw_spin_unlock_irq(&dl_b->lock); + if (!hrtimer_active(&p->dl.inactive_timer)) { + struct dl_bw *dl_b = dl_bw_of(task_cpu(p)); + + /* + * If the "inactive timer is not active, the 0-lag time + * is already passed, so we immediately decrease the + * total deadline utilization + */ + raw_spin_lock_irq(&dl_b->lock); + __dl_clear(dl_b, p->dl.dl_bw); + raw_spin_unlock_irq(&dl_b->lock); + } } static void set_curr_task_dl(struct rq *rq) -- 2.5.0