From patchwork Fri Nov 27 11:26:47 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juri Lelli X-Patchwork-Id: 57363 Delivered-To: patch@linaro.org Received: by 10.112.155.196 with SMTP id vy4csp1076146lbb; Fri, 27 Nov 2015 03:26:28 -0800 (PST) X-Received: by 10.98.11.133 with SMTP id 5mr47542406pfl.26.1448623588199; Fri, 27 Nov 2015 03:26:28 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w63si3828001pfa.222.2015.11.27.03.26.27; Fri, 27 Nov 2015 03:26:28 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754325AbbK0L00 (ORCPT + 28 others); Fri, 27 Nov 2015 06:26:26 -0500 Received: from foss.arm.com ([217.140.101.70]:49154 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750978AbbK0L0X (ORCPT ); Fri, 27 Nov 2015 06:26:23 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D40C149; Fri, 27 Nov 2015 03:26:04 -0800 (PST) Received: from e106622-lin (e106622-lin.cambridge.arm.com [10.1.208.152]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 309313F2E5; Fri, 27 Nov 2015 03:26:22 -0800 (PST) Date: Fri, 27 Nov 2015 11:26:47 +0000 From: Juri Lelli To: Wanpeng li Cc: Ingo Molnar , Peter Zijlstra , linux-kernel@vger.kernel.org, Luca Abeni Subject: Re: [PATCH v3] sched/deadline: fix earliest_dl.next logic Message-ID: <20151127112647.GR20439@e106622-lin> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, [+Luca, as he has been testing this patch an he has probably findings to share] On 19/11/15 18:11, Wanpeng li wrote: > earliest_dl.next should cache deadline of the earliest ready task that > is also enqueued in the pushable rbtree, as pull algorithm uses this > information to find candidates for migration: if the earliest_dl.next > deadline of source rq is earlier than the earliest_dl.curr deadline of > destination rq, the task from the source rq can be pulled. > > However, current implementation only guarantees that earliest_dl.next is > the deadline of the next ready task instead of the next pushable task; > which will result in potentially holding both rqs' lock and find nothing > to migrate because of affinity constraints. In addition, current logic > doesn't update the next candidate for pushing in pick_next_task_dl(), > even if the running task is never eligible. > > This patch fixes both problems by updating earliest_dl.next when > pushable dl task is enqueued/dequeued, similar to what we already do for > RT. > > Signed-off-by: Wanpeng li > --- > v2 -> v3: > * reset dl_rq->earliest_dl.next to 0 if !next_pushable > v1 -> v2: > * fix potential NULL pointer dereference > > kernel/sched/deadline.c | 63 ++++++++++--------------------------------------- > 1 file changed, 12 insertions(+), 51 deletions(-) > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index 142df26..547d102 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -87,6 +87,8 @@ void init_dl_rq(struct dl_rq *dl_rq) > > #ifdef CONFIG_SMP > > +static struct task_struct *pick_next_pushable_dl_task(struct rq *rq); > + > static inline int dl_overloaded(struct rq *rq) > { > return atomic_read(&rq->rd->dlo_count); > @@ -181,11 +183,15 @@ static void enqueue_pushable_dl_task(struct rq *rq, struct task_struct *p) > > rb_link_node(&p->pushable_dl_tasks, parent, link); > rb_insert_color(&p->pushable_dl_tasks, &dl_rq->pushable_dl_tasks_root); > + > + if (dl_time_before(p->dl.deadline, dl_rq->earliest_dl.next)) > + dl_rq->earliest_dl.next = p->dl.deadline; This seems to be a bug, as earliest_dl.next is initialized to 0 and dl_time_before() will say that p has later deadline than earliest_dl.next even if p is actually the first pushable task. > } > > static void dequeue_pushable_dl_task(struct rq *rq, struct task_struct *p) > { > struct dl_rq *dl_rq = &rq->dl; > + struct task_struct *next_pushable; > > if (RB_EMPTY_NODE(&p->pushable_dl_tasks)) > return; > @@ -199,6 +205,12 @@ static void dequeue_pushable_dl_task(struct rq *rq, struct task_struct *p) > > rb_erase(&p->pushable_dl_tasks, &dl_rq->pushable_dl_tasks_root); > RB_CLEAR_NODE(&p->pushable_dl_tasks); > + > + next_pushable = pick_next_pushable_dl_task(rq); > + if (next_pushable) > + dl_rq->earliest_dl.next = next_pushable->dl.deadline; > + else > + dl_rq->earliest_dl.next = 0; As already said, this is useless (sorry for suggesting it in the first instance). What follows might fix these two issue. However, Luca is telling me that he is seeing some other issue with this patch on his testing box. Maybe he can directly comment on this. Thanks, - Juri --- kernel/sched/deadline.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) -- -- 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/ diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 547d102..d6de660 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -178,14 +178,13 @@ static void enqueue_pushable_dl_task(struct rq *rq, struct task_struct *p) } } - if (leftmost) + if (leftmost) { dl_rq->pushable_dl_tasks_leftmost = &p->pushable_dl_tasks; + dl_rq->earliest_dl.next = p->dl.deadline; + } rb_link_node(&p->pushable_dl_tasks, parent, link); rb_insert_color(&p->pushable_dl_tasks, &dl_rq->pushable_dl_tasks_root); - - if (dl_time_before(p->dl.deadline, dl_rq->earliest_dl.next)) - dl_rq->earliest_dl.next = p->dl.deadline; } static void dequeue_pushable_dl_task(struct rq *rq, struct task_struct *p) @@ -209,8 +208,6 @@ static void dequeue_pushable_dl_task(struct rq *rq, struct task_struct *p) next_pushable = pick_next_pushable_dl_task(rq); if (next_pushable) dl_rq->earliest_dl.next = next_pushable->dl.deadline; - else - dl_rq->earliest_dl.next = 0; } static inline int has_pushable_dl_tasks(struct rq *rq)