Skip to contentSkip to conversation

sched/rt: Decrease number of calls of push_rt_task() in push_rt_tasks()

PATCH In review Kirill Tkhai 3 participants 4 messages Jan 27, 2013 – Feb 5, 2013 Scheduler

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sched/rt: Decrease number of calls of push_rt_task() in push_rt_tasks()

The patch aims to decrease the number of calls of push_rt_task() in push_rt_tasks().

It's not necessary to push more than 'num_online_cpus() - 1' tasks. If just pushed task doesn't leave its new CPU during our local call of push_rt_tasks() than we won't push another task to the CPU. If it leave or change priority than it will pull new task by itself.

CC Steven Rostedt <[email protected]>
CC Ingo Molnar <[email protected]>
CC Peter Zijlstra <[email protected]>
CC linux-rt-users <[email protected]>
---
 kernel/sched/rt.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
kernel/sched/rt.c +3 -1
index 4e8f0f4..edf046d 100644
@@ -1703,8 +1703,10 @@ out:
 
 static void push_rt_tasks(struct rq *rq)
 {
+	int num_cpus = num_online_cpus() - 1;
+
 	/* push_rt_task will return true if it moved an RT */
-	while (push_rt_task(rq))
+	while (push_rt_task(rq) && --num_cpus > 0)
 		;
 }
 
#1 lore raw
Quoted 7 lines · Kirill Tkhai

The patch aims to decrease the number of calls of push_rt_task() in push_rt_tasks().

It's not necessary to push more than 'num_online_cpus() - 1' tasks. If just pushed task doesn't leave its new CPU during our local call of push_rt_tasks() than we won't push another task to the CPU. If it leave or change priority than it will pull new task by itself.

I'm curious. Have you hit situations where this was an issue? Or was this just discovered by code review?

-- Steve

Quoted 25 lines
CC Steven Rostedt <[email protected]>
CC Ingo Molnar <[email protected]>
CC Peter Zijlstra <[email protected]>
CC linux-rt-users <[email protected]>
---
 kernel/sched/rt.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
kernel/sched/rt.c +3 -1
index 4e8f0f4..edf046d 100644
@@ -1703,8 +1703,10 @@ out:
 
 static void push_rt_tasks(struct rq *rq)
 {
+	int num_cpus = num_online_cpus() - 1;
+
 	/* push_rt_task will return true if it moved an RT */
-	while (push_rt_task(rq))
+	while (push_rt_task(rq) && --num_cpus > 0)
 		;
 }
 
#2 lore raw

31.01.2013, 20:08, "Steven Rostedt" <[email protected]>:

Quoted 5 lines · Kirill Tkhai
Quoted 4 lines

 The patch aims to decrease the number of calls of push_rt_task() in push_rt_tasks().

Quoted 7 lines
Quoted 4 lines

 It's not necessary to push more than 'num_online_cpus() - 1' tasks. If just pushed task doesn't leave its new CPU during our local call of push_rt_tasks() than we won't push another task to the CPU. If it leave or change priority than it will pull new task by itself.

I'm curious. Have you hit situations where this was an issue? Or was this just discovered by code review?

No, I did't hit this situation. It's impossible to hook every situation.

Thanks for your explanation.

Kirill

Quoted 26 lines

-- Steve

Quoted 23 lines
CC Steven Rostedt <[email protected]>
CC Ingo Molnar <[email protected]>
CC Peter Zijlstra <[email protected]>
CC linux-rt-users <[email protected]>

 ---
  kernel/sched/rt.c |    4 +++-

  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 4e8f0f4..edf046d 100644
 --- a/kernel/sched/rt.c
 +++ b/kernel/sched/rt.c
 @@ -1703,8 +1703,10 @@ out:

  static void push_rt_tasks(struct rq *rq)
  {
 + int num_cpus = num_online_cpus() - 1;
 +
          /* push_rt_task will return true if it moved an RT */
 - while (push_rt_task(rq))
 + while (push_rt_task(rq) && --num_cpus > 0)
                  ;
  }

#3 lore raw
Quoted 13 lines · Kirill Tkhai

31.01.2013, 20:08, "Steven Rostedt" <[email protected]>:

Quoted 10 lines · Kirill Tkhai
Quoted 8 lines

The patch aims to decrease the number of calls of push_rt_task() in push_rt_tasks().

It's not necessary to push more than 'num_online_cpus() - 1' tasks. If just pushed task doesn't leave its new CPU during our local call of push_rt_tasks() than we won't push another task to the CPU. If it leave or change priority than it will pull new task by itself.

Quoted 9 lines
Quoted 2 lines

I'm curious. Have you hit situations where this was an issue? Or was this just discovered by code review?

No, I did't hit this situation. It's impossible to hook every situation.

Thanks for your explanation.

Kirill

Suppose we have a large number of cpus(say 4096), with the last one running a low-priority task on it. Is it possible with this patch we will never reach the last cpu in case that previous cpu has complete the pulled task?