Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753647AbcDUQD0 (ORCPT ); Thu, 21 Apr 2016 12:03:26 -0400 Received: from merlin.infradead.org ([205.233.59.134]:39478 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753315AbcDUQDY (ORCPT ); Thu, 21 Apr 2016 12:03:24 -0400 Date: Thu, 21 Apr 2016 18:03:15 +0200 From: Peter Zijlstra To: Wanpeng Li Cc: Rik van Riel , Chris Metcalf , Frederic Weisbecker , Christoph Lameter , Ingo Molnar , Luiz Capitulino , Thomas Gleixner , Viresh Kumar , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] nohz_full: Make sched_should_stop_tick() more conservative Message-ID: <20160421160315.GK24771@twins.programming.kicks-ass.net> References: <1459539771-4251-1-git-send-email-cmetcalf@mellanox.com> <1459797143.6219.22.camel@redhat.com> <20160421144213.GN3408@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160421144213.GN3408@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2008 Lines: 63 On Thu, Apr 21, 2016 at 04:42:13PM +0200, Peter Zijlstra wrote: > So I think that is indeed the right thing here. But looking at this > function I think there's more problems with it. > > It seems to assume that if there's FIFO tasks, those will run. This is > incorrect. The FIFO task can have a lower prio than an RR task, in which > case the RR task will run. > > So the whole fifo_nr_running test seems misplaced, it should go after > the rr_nr_running tests. That is, only if !rr_nr_running, can we use > fifo_nr_running like this. A little something like so perhaps; can anybody test? --- diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ffec7d9e7763..4240686f6857 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -596,17 +596,8 @@ bool sched_can_stop_tick(struct rq *rq) return false; /* - * FIFO realtime policy runs the highest priority task (after DEADLINE). - * Other runnable tasks are of a lower priority. The scheduler tick - * isn't needed. - */ - fifo_nr_running = rq->rt.rt_nr_running - rq->rt.rr_nr_running; - if (fifo_nr_running) - return true; - - /* - * Round-robin realtime tasks time slice with other tasks at the same - * realtime priority. + * If there are more than one RR tasks, we need the tick to effect the + * actual RR behaviour. */ if (rq->rt.rr_nr_running) { if (rq->rt.rr_nr_running == 1) @@ -615,8 +606,20 @@ bool sched_can_stop_tick(struct rq *rq) return false; } - /* Normal multitasking need periodic preemption checks */ - if (rq->cfs.nr_running > 1) + /* + * If there's no RR tasks, but FIFO tasks, we can skip the tick, no + * forced preemption between FIFO tasks. + */ + fifo_nr_running = rq->rt.rt_nr_running - rq->rt.rr_nr_running; + if (fifo_nr_running) + return true; + + /* + * If there are no DL,RR/FIFO tasks, there must only be CFS tasks left; + * if there's more than one we need the tick for involuntary + * preemption. + */ + if (rq->nr_running > 1) return false; return true;