Re: [PATCH v2] sched/fair: WARN and refuse to set buddy when !se->on_rq

From: Konstantin Khlebnikov
Date: Tue Jun 20 2017 - 03:41:46 EST




On 19.06.2017 17:05, Daniel Axtens wrote:
Hi Konstantin and Peter,

Just checking if this version was OK with you - I hadn't heard anything
and I noticed it's not in -next so I just wanted to check to see if
there were any other changes you wanted.

Looks good for me. Exactly that debug helped me alot at that time.


Regards,
Daniel


Daniel Axtens <dja@xxxxxxxxxx> writes:

If we set a next or last buddy for a se that is not on_rq, we will
end up taking a NULL pointer dereference in wakeup_preempt_entity
via pick_next_task_fair.

Detect when we would be about to do that, throw a warning and
then refuse to actually set it.

This has been suggested at least twice[0][1]: just do it.

[0] https://marc.info/?l=linux-kernel&m=146651668921468&w=2
[1] https://lkml.org/lkml/2016/6/16/663

Cc: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx>
Cc: Ben Segall <bsegall@xxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Signed-off-by: Daniel Axtens <dja@xxxxxxxxxx>

---

I recently had to debug a problem with these (we hadn't backported
Konstantin's patches in this area) and this would have saved a lot
of time/pain.

v2: use SCHED_WARN_ON to restrict when the test is run. This is a
macro for WARN_ON_ONCE, which is convenient.
---
kernel/sched/fair.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d71109321841..44b94cfe02cb 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6168,8 +6168,11 @@ static void set_last_buddy(struct sched_entity *se)
if (entity_is_task(se) && unlikely(task_of(se)->policy == SCHED_IDLE))
return;
- for_each_sched_entity(se)
+ for_each_sched_entity(se) {
+ if (SCHED_WARN_ON(!se->on_rq))
+ return;
cfs_rq_of(se)->last = se;
+ }
}
static void set_next_buddy(struct sched_entity *se)
@@ -6177,8 +6180,11 @@ static void set_next_buddy(struct sched_entity *se)
if (entity_is_task(se) && unlikely(task_of(se)->policy == SCHED_IDLE))
return;
- for_each_sched_entity(se)
+ for_each_sched_entity(se) {
+ if (SCHED_WARN_ON(!se->on_rq))
+ return;
cfs_rq_of(se)->next = se;
+ }
}
static void set_skip_buddy(struct sched_entity *se)
--
2.11.0