Received-SPF: pass (google.com: domain of linux-kernel+bounces-101015-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
To: linux-kernel@vger.kernel.org,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>,
	Mel Gorman <mgorman@suse.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Valentin Schneider <vschneid@redhat.com>
Cc: Suleiman Souhlal <suleiman@google.com>,
	Youssef Esmat <youssefesmat@google.com>,
	David Vernet <void@manifault.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	joseph.salisbury@canonical.com,
	Luca Abeni <luca.abeni@santannapisa.it>,
	Tommaso Cucinotta <tommaso.cucinotta@santannapisa.it>,
	Vineeth Pillai <vineeth@bitbyteword.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Phil Auld <pauld@redhat.com>,
	"Joel Fernandes (Google)" <joel@joelfernandes.org>
Subject: [PATCH v2 15/15] sched/deadline: Always start a new period if CFS exceeded DL runtime
Date: Tue, 12 Mar 2024 21:24:51 -0400
Message-Id: <20240313012451.1693807-16-joel@joelfernandes.org>
In-Reply-To: <20240313012451.1693807-1-joel@joelfernandes.org>
References: <20240313012451.1693807-1-joel@joelfernandes.org>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

We believe that this is the right thing to do. The unit test
(cs_dlserver_test) also agrees. If we let the CFS run without starting a
new period, while the server is regularly throttled, then the test fails
because CFS does not appear to get enough bandwidth.

Intuitively, this makes sense to do as well. If CFS used up all the CFS
bandwidth, while the DL server was in a throttled state, it got the
bandwidth it wanted and some. Now, we can start all over from scratch to
guarantee it a minimum bandwidth.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 kernel/sched/deadline.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 179369d27f66..a0ea668ac1bf 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1454,23 +1454,6 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
 	 * starting a new period, pushing the activation to the zero-lax time.
 	 */
 	if (dl_se->dl_defer && dl_se->dl_throttled && dl_runtime_exceeded(dl_se)) {
-		s64 runtime_diff = dl_se->runtime + dl_se->dl_runtime;
-
-		/*
-		 * If this is a regular throttling case, let it run negative until
-		 * the dl_runtime - runtime > 0. The reason being is that the next
-		 * replenishment will result in a positive runtime one period ahead.
-		 *
-		 * Otherwise, the deadline will be pushed more than one period, not
-		 * providing runtime/period anymore.
-		 *
-		 * If the dl_runtime - runtime < 0, then the server was able to get
-		 * the runtime/period before the replenishment. So it is safe
-		 * to start a new deffered period.
-		 */
-		if (!dl_se->dl_defer_armed && runtime_diff > 0)
-			return;
-
 		hrtimer_try_to_cancel(&dl_se->dl_timer);
 
 		replenish_dl_new_period(dl_se, dl_se->rq);
-- 
2.34.1