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

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

[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.

Signed-off-by: Kirill V Tkhai <tkhai@yandex.ru>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: linux-rt-users <linux-rt-users@vger.kernel.org>
---
 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)
 		;
 }
 

Re: [PATCH] sched/rt: Decrease number of calls of push_rt_task() in push_rt_tasks()

[Steven Rostedt]

On Mon, 2013-01-28 at 03:46 +0400, Kirill Tkhai wrote:
> 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

> 
> Signed-off-by: Kirill V Tkhai <tkhai@yandex.ru>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@kernel.org>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: linux-rt-users <linux-rt-users@vger.kernel.org>
> ---
>  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)
>  		;
>  }
>  

Re: [PATCH] sched/rt: Decrease number of calls of push_rt_task() in push_rt_tasks()

[Kirill Tkhai]

31.01.2013, 20:08, "Steven Rostedt" <rostedt@goodmis.org>:
> On Mon, 2013-01-28 at 03:46 +0400, Kirill Tkhai wrote:
>
>>  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?

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

Thanks for your explanation.

Kirill

>
> -- Steve
>
>>  Signed-off-by: Kirill V Tkhai <tkhai@yandex.ru>
>>  CC: Steven Rostedt <rostedt@goodmis.org>
>>  CC: Ingo Molnar <mingo@kernel.org>
>>  CC: Peter Zijlstra <peterz@infradead.org>
>>  CC: linux-rt-users <linux-rt-users@vger.kernel.org>
>>  ---
>>   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)
>>                   ;
>>   }

Re: [PATCH] sched/rt: Decrease number of calls of push_rt_task() in push_rt_tasks()

[Honghui Zhang]

On 2013/2/1 5:57, Kirill Tkhai wrote:
> 
> 
> 31.01.2013, 20:08, "Steven Rostedt" <rostedt@goodmis.org>:
>> On Mon, 2013-01-28 at 03:46 +0400, Kirill Tkhai wrote:
>>
>>>  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?
> 
> 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?

Re:[PATCH] sched/rt: Decrease number of calls of push_rt_task() in push_rt_tasks()

[Kirill Tkhai]

> On 2013/2/1 5:57, Kirill Tkhai wrote:
> 
>> 31.01.2013, 20:08, "Steven Rostedt" <rostedt@goodmis.org>:
>>
>>> On Mon, 2013-01-28 at 03:46 +0400, Kirill Tkhai wrote:
>>>
>>>> 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?
>>
>> 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?

Yes. But this patch is about several pushable tasks on the same cpu.

Kirill

 -- 

Re: [PATCH] sched/rt: Decrease number of calls of push_rt_task() in push_rt_tasks()

[Honghui Zhang]

On 2013/2/5 15:22, Kirill Tkhai wrote:
>>
>> 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?
> 
> Yes. But this patch is about several pushable tasks on the same cpu.
> 
> Kirill
> 

Maybe I haven't make myself understood, but in that case, there will be a higher-priority
task in the runqueue of the pushing cpu, and a lower-priority task running in the last cpu
which could not be preempt by push_rt_task(),  I don't think it's acceptable.

Hans