From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753624AbbBDDRF (ORCPT ); Tue, 3 Feb 2015 22:17:05 -0500 Received: from cdptpa-outbound-snat.email.rr.com ([107.14.166.226]:32909 "EHLO cdptpa-oedge-vip.email.rr.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751753AbbBDDRB (ORCPT ); Tue, 3 Feb 2015 22:17:01 -0500 Date: Tue, 3 Feb 2015 22:17:38 -0500 From: Steven Rostedt To: Xunlei Pang Cc: linux-kernel@vger.kernel.org, Peter Zijlstra , Juri Lelli , Xunlei Pang Subject: Re: [PATCH RESEND 2/2] sched/rt: Add check_preempt_equal_prio() logic in pick_next_task_rt() Message-ID: <20150203221738.63895464@grimm.local.home> In-Reply-To: <1423012341-30265-2-git-send-email-xlpang@126.com> References: <1423012341-30265-1-git-send-email-xlpang@126.com> <1423012341-30265-2-git-send-email-xlpang@126.com> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-RR-Connecting-IP: 107.14.168.130:25 X-Cloudmark-Score: 0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 4 Feb 2015 09:12:21 +0800 Xunlei Pang wrote: > From: Xunlei Pang > > check_preempt_curr() doesn't call sched_class::check_preempt_curr > when the class of current is a higher level. So if there is a DL > task running when doing this for RT, check_preempt_equal_prio() > will definitely miss, which may result in some response latency > for this RT task if it is pinned and there're some same-priority > migratable rt tasks already queued. > > We should do the similar thing in select_task_rq_rt() when first > picking rt tasks after running out of DL tasks. > > This patch tackles the issue by peeking the next rt task(RT1), and > if find RT1 migratable, just requeue it to the tail of the rq using > requeue_task_rt(rq, p, 0). In this way: > - If there do have another rt task(RT2) with the same priority as > RT1, RT2 will finally be picked as the running task. While RT1 > will be pushed onto another cpu via RT1's post_schedule(), as > RT1 is migratable. The difference from check_preempt_equal_prio() > here is that we just don't care whether RT2 is migratable. > > - Otherwise, if there's no rt task with the same priority as RT1, > RT1 will still be picked as the running task after the requeuing. > > Signed-off-by: Xunlei Pang > --- > kernel/sched/rt.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > index 4dacb6e..b2385ee 100644 > --- a/kernel/sched/rt.c > +++ b/kernel/sched/rt.c > @@ -1477,6 +1477,21 @@ pick_next_task_rt(struct rq *rq, struct task_struct *prev) > > put_prev_task(rq, prev); > > +#ifdef CONFIG_SMP > + /* > + * If there's a running deadline task, check_preempt_curr() > + * doesn't invoke check_preempt_curr_rt() for rt tasks, so > + * we can do it here. > + */ Why the strange indentation? > + if (prev->sched_class == &dl_sched_class && > + rq->rt.rt_nr_total > 1) { > + p = _pick_next_task_rt(rq, 1); /* peek only */ I hate the "peek only". Just split the function into two, where you have something like check_next_task(rq) which does your "peek only" and the __pick_next_task_rt() calls check_next_task() first and then runs the rest of the code. -- Steve > + if (p->nr_cpus_allowed != 1 && > + cpupri_find(&rq->rd->cpupri, p, NULL)) > + requeue_task_rt(rq, p, 0); > + } > +#endif > + > p = _pick_next_task_rt(rq, 0); > > /* The running task is never eligible for pushing */