lkml.org 
[lkml]   [2013]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: false nr_running check in load balance?
On Tue, Aug 06, 2013 at 09:23:46PM +0800, Lei Wen wrote:
> Hi Paul,
>
> I notice in load_balance function, it would check busiest->nr_running
> to decide whether to perform the real task movement.
>
> But in some case, I saw the nr_running is not matching with
> the task in the queue, which seems make scheduler to do many redundant
> checking.
> What I means is like there is only one task in the queue, but nr_running
> shows it has two. So if that task cannot be moved, it would be still checked
> for twice.
>
> With further checking, I find there is one patch you submit before:
> commit 953bfcd10e6f3697233e8e5128c611d275da39c1
> Author: Paul Turner <pjt@google.com>
> Date: Thu Jul 21 09:43:27 2011 -0700
>
> sched: Implement hierarchical task accounting for SCHED_OTHER
>
> In this patch, you increase nr_running when enqueue enqueue_task_stop,
> which is the reason nr_running is increase while task not be increased.
> It is true at that time, the stopper has been waken up and enqueue again
> into cpu, and do the migration job. So the logic should be right there.
>
> My question is whether we could change the judgment into cfs_rq->nr_running?
> Since the load_balance is only for cfs, right?
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index bb456f4..ffc0d35 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5096,7 +5096,7 @@ redo:
> schedstat_add(sd, lb_imbalance[idle], env.imbalance);
>
> ld_moved = 0;
> - if (busiest->nr_running > 1) {
> + if (busiest->cfs.nr_running > 1) {
> /*
> * Attempt to move tasks. If find_busiest_group has found
> * an imbalance but busiest->nr_running <= 1, the group is
>

Not quite right; I think you need busiest->cfs.h_nr_running.
cfs.nr_running is the number of entries running in this 'group'. If
you've got nested groups like:

'root'
\
'A'
/ \
t1 t2

root.nr_running := 1 'A', even though you've got multiple running tasks.




\
 
 \ /
  Last update: 2013-08-12 17:01    [W:0.066 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site