[PATCH] [RFC]: sched/deadline: Avoid double enqueue_pushable_dl_task() warning

[John Stultz]

In testing with the full Proxy Execution patch stack, I found
I would occasionally trip over the !RB_EMPTY_NODE() WARN_ON in
enqueue_pushable_dl_task(), where the task we're adding to the
pushable list is already enqueued.

This triggers from put_prev_task_dl(), where it seems we go into
put_prev_task_dl()
-> update_curr_dl()
   -> update_curr_dl_se() [hitting the dl_runtime_exceeded() case]
      -> enqueue_task_dl()
         -> enqueue_pushable_dl_task()

Adding the task to the pushable the first time.

Then we back up the call stack to put_prev_task_dl(), which at
the end again calls enqueue_pushable_dl_task(), trying to add it
a second time, tripping the warning.

To avoid this, add a dl_task_pushable() helper which we can use
to replace the RB_EMPTY_NODE checks elsewhere, and then before
enqueueing in put_prev_task_dl(), we can first check
dl_task_pushable() to avoid the double enqueue.

I've only really tripped this while stress testing the full
Proxy Execution patch series, and I haven't had time to do the
same level of stress testing with vanilla, so its possible this
isn't likely to occur with the current upstream, but it seems
like it could happen, so I wanted to send this out for comment.

Fixes: 63ba8422f876e ("sched/deadline: Introduce deadline servers")
Signed-off-by: John Stultz <jstultz@xxxxxxxxxx>
---
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Juri Lelli <juri.lelli@xxxxxxxxxx>
Cc: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
Cc: Dietmar Eggemann <dietmar.eggemann@xxxxxxx>
CC: Steven Rostedt <rostedt@xxxxxxxxxxx>
Cc: Ben Segall <bsegall@xxxxxxxxxx>
Cc: Mel Gorman <mgorman@xxxxxxx>
Cc: Valentin Schneider <vschneid@xxxxxxxxxx>
Cc: Suleiman Souhlal <suleiman@xxxxxxxxxx>
Cc: K Prateek Nayak <kprateek.nayak@xxxxxxx>
Cc: kernel-team@xxxxxxxxxxx
---
 kernel/sched/deadline.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index d08b004293234..7a97147560eb3 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -571,6 +571,11 @@ static inline int has_pushable_dl_tasks(struct rq *rq)
 	return !RB_EMPTY_ROOT(&rq->dl.pushable_dl_tasks_root.rb_root);
 }
 
+static inline bool dl_task_pushable(struct task_struct *p)
+{
+	return !RB_EMPTY_NODE(&p->pushable_dl_tasks);
+}
+
 /*
  * The list of pushable -deadline task is not a plist, like in
  * sched_rt.c, it is an rb-tree with tasks ordered by deadline.
@@ -579,7 +584,7 @@ static void enqueue_pushable_dl_task(struct rq *rq, struct task_struct *p)
 {
 	struct rb_node *leftmost;
 
-	WARN_ON_ONCE(!RB_EMPTY_NODE(&p->pushable_dl_tasks));
+	WARN_ON_ONCE(dl_task_pushable(p));
 
 	leftmost = rb_add_cached(&p->pushable_dl_tasks,
 				 &rq->dl.pushable_dl_tasks_root,
@@ -599,7 +604,7 @@ static void dequeue_pushable_dl_task(struct rq *rq, struct task_struct *p)
 	struct rb_root_cached *root = &dl_rq->pushable_dl_tasks_root;
 	struct rb_node *leftmost;
 
-	if (RB_EMPTY_NODE(&p->pushable_dl_tasks))
+	if (!dl_task_pushable(p))
 		return;
 
 	leftmost = rb_erase_cached(&p->pushable_dl_tasks, root);
@@ -2646,6 +2651,21 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_s
 	if (task_is_blocked(p))
 		return;
 
+	/*
+	 * its possible the following call chain from
+	 * update_curr_dl() called above has already
+	 * added us to the pushable list:
+	 * update_curr_dl()
+	 * -> update_curr_dl_se()
+	 *    -> enqueue_task_dl()
+	 *       -> enqueue_pushable_dl_task()
+	 *
+	 * So check dl_task_pushable() first to make sure we don't
+	 * get added twice
+	 */
+	if (dl_task_pushable(p))
+		return;
+
 	if (on_dl_rq(&p->dl) && p->nr_cpus_allowed > 1)
 		enqueue_pushable_dl_task(rq, p);
 }
-- 
2.53.0.473.g4a7958ca14-goog