Skip to contentSkip to conversation

sched/rt.c: leaf_rt_rq_list contains empty queues. Fix it

PATCH In review Kirill Tkhai 2 participants 5 messages Sep 21, 2012 Scheduler

lore
Share & tools
lore

Kirill Tkhai

author
#0 lore raw

PATCH 1/1 sched/rt.c: leaf_rt_rq_list contains empty queues. Fix it

Function __enqueue_rt_entity() adds an empty queue to leaf_rt_rq_list.

So, pick_next_highest_task_rt() picks empty queues. Fix it.

Signed-off-by Kirill Tkhai <[email protected]>
---
 kernel/sched/rt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/sched/rt.c +1 -1
index 944cb68..b07edb9 100644
@@ -1103,7 +1103,7 @@ static void __enqueue_rt_entity(struct sched_rt_entity *rt_se, bool head)
 	if (group_rq && (rt_rq_throttled(group_rq) || !group_rq->rt_nr_running))
 		return;
 
-	if (!rt_rq->rt_nr_running)
+	if (rt_rq->rt_nr_running)
 		list_add_leaf_rt_rq(rt_rq);
 
 	if (head)
author
#1 lore raw

Second version. Add every not empty queue once. The patch:

Function __enqueue_rt_entity() adds an empty queue to leaf_rt_rq_list.

So, pick_next_highest_task_rt() picks empty queues. Fix it.

Signed-off-by Kirill Tkhai <[email protected]>
---
 kernel/sched/rt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/sched/rt.c +1 -1
index 944cb68..97a0954 100644
@@ -1103,7 +1103,7 @@ static void __enqueue_rt_entity(struct sched_rt_entity *rt_se, bool head)
 	if (group_rq && (rt_rq_throttled(group_rq) || !group_rq->rt_nr_running))
 		return;
 
-	if (!rt_rq->rt_nr_running)
+	if (rt_rq->rt_nr_running == 1)
 		list_add_leaf_rt_rq(rt_rq);
 
 	if (head)
reviewer
#2 lore raw
Quoted 15 lines · Kirill Tkhai

Second version. Add every not empty queue once. The patch:

Function __enqueue_rt_entity() adds an empty queue to leaf_rt_rq_list.

So, pick_next_highest_task_rt() picks empty queues. Fix it.

Signed-off-by Kirill Tkhai <[email protected]>
---
 kernel/sched/rt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/sched/rt.c +0 -0
index 944cb68..97a0954 100644

kernel/sched/rt.c static void __enqueue_rt_entity(struct sched_rt_entity *rt_se, bool head) · line 1107

Quoted 6 lines
kernel/sched/rt.c +1 -1
@@ -1103,7 +1103,7 @@ static void __enqueue_rt_entity(struct sched_rt_entity *rt_se, bool head)
 	if (group_rq && (rt_rq_throttled(group_rq) || !group_rq->rt_nr_running))
 		return;
 
-	if (!rt_rq->rt_nr_running)
+	if (rt_rq->rt_nr_running == 1)
 		list_add_leaf_rt_rq(rt_rq);

Again, the rt_nr_running hasn't been incremented yet. This patch will add it when the rq gets a second task.

-- Steve

Quoted 2 lines
kernel/sched/rt.c +0 -0
 	if (head)
author
#3 lore raw
Quoted 4 lines

Again, the rt_nr_running hasn't been incremented yet. This patch will add it when the rq gets a second task.

-- Steve

Right, thanks for the explanation

Kirill

reviewer
#4 lore raw
In reply to Kirill Tkhai
Quoted 3 lines · Kirill Tkhai

Function __enqueue_rt_entity() adds an empty queue to leaf_rt_rq_list.

So, pick_next_highest_task_rt() picks empty queues. Fix it.

Do you have an example where this breaks?

Quoted 10 lines
Signed-off-by Kirill Tkhai <[email protected]>
---
 kernel/sched/rt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/sched/rt.c +0 -0
index 944cb68..b07edb9 100644

kernel/sched/rt.c static void __enqueue_rt_entity(struct sched_rt_entity *rt_se, bool head) · line 1107

Quoted 6 lines
kernel/sched/rt.c +1 -1
@@ -1103,7 +1103,7 @@ static void __enqueue_rt_entity(struct sched_rt_entity *rt_se, bool head)
 	if (group_rq && (rt_rq_throttled(group_rq) || !group_rq->rt_nr_running))
 		return;
 
-	if (!rt_rq->rt_nr_running)
+	if (rt_rq->rt_nr_running)
 		list_add_leaf_rt_rq(rt_rq);

Note, the list_add_leaf_rt_rq() is called when the first task is queued up. The function here is __enqueue_rt_entity(), which will increment rt_rq->rt_nr_running. If this is the first entry we add the queue now, as the __dequeue_rt_entity() does the list_del_leaf_rt_rq() when it goes to zero.

-- Steve

Quoted 2 lines
kernel/sched/rt.c +0 -0
 	if (head)