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;
From:   =?UTF-8?q?Jan=20H=2E=20Sch=C3=B6nherr?= <jschoenh@amazon.de>
To:     Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>
Cc:     =?UTF-8?q?Jan=20H=2E=20Sch=C3=B6nherr?= <jschoenh@amazon.de>,
        linux-kernel@vger.kernel.org
Subject: [RFC 51/60] cosched: Hacky work-around to avoid observing zero weight SD-SE
Date:   Fri,  7 Sep 2018 23:40:38 +0200
Message-Id: <20180907214047.26914-52-jschoenh@amazon.de>
In-Reply-To: <20180907214047.26914-1-jschoenh@amazon.de>
References: <20180907214047.26914-1-jschoenh@amazon.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

The aggregated SD-SE weight is updated lock-free to avoid contention
on the higher level. This also means, that we have to be careful
with intermediate values as another CPU could pick up the value and
perform actions based on it.

Within reweight_entity() there is such a place, where weight is removed,
locally modified, and added back. If another CPU locks the higher level
and observes a zero weight, it will perform incorrect decisions when
it is dequeuing a task: it won't stop dequeuing, although there is still
load in one of the child runqueues.

Prevent this from happening by temporarily bumping the aggregated value.

(A nicer solution would be to apply only the actual difference to the
aggregate instead of doing full removal and a subsequent addition.)

Signed-off-by: Jan H. Schönherr <jschoenh@amazon.de>
---
 kernel/sched/fair.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1eee262ecf88..483db54ee20a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2865,6 +2865,16 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
 		/* commit outstanding execution time */
 		if (cfs_rq->curr == se)
 			update_curr(cfs_rq);
+#ifdef CONFIG_COSCHEDULING
+		/*
+		 * FIXME: Temporarily adjust the sdse_load to prevent a zero
+		 *        value from being visible over the calls to
+		 *        account_entity_dequeue()/account_entity_enqueue().
+		 *        It leads to incorrect decisions in scheduler code.
+		 */
+		if (!cfs_rq->sdrq.is_root && !cfs_rq->throttled)
+			atomic64_add(NICE_0_LOAD, &cfs_rq->sdrq.sd_parent->sdse_load);
+#endif
 		account_entity_dequeue(cfs_rq, se);
 		dequeue_runnable_load_avg(cfs_rq, se);
 	}
@@ -2886,6 +2896,11 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
 	enqueue_load_avg(cfs_rq, se);
 	if (se->on_rq) {
 		account_entity_enqueue(cfs_rq, se);
+#ifdef CONFIG_COSCHEDULING
+		/* FIXME: see above */
+		if (!cfs_rq->sdrq.is_root && !cfs_rq->throttled)
+			atomic64_sub(NICE_0_LOAD, &cfs_rq->sdrq.sd_parent->sdse_load);
+#endif
 		enqueue_runnable_load_avg(cfs_rq, se);
 	}
 }
-- 
2.9.3.1.gcba166c.dirty