Re: sched:

[Peter Zijlstra <a.p.zijlstra@chello.nl>]

On Thu, 2008-09-25 at 09:51 +0530, Bharata B Rao wrote:
> Hi,
> 
> sched: Maintain only task entities in cfs_rq->tasks list.
> 
> cfs_rq->tasks list is used by the load balancer to iterate
> over all the tasks. Currently it holds all the entities
> (both task and group entities) because of which there is
> a need to check for group entities explicitly during load
> balancing. This patch changes the cfs_rq->tasks list to
> hold only task entities.

Looks fine. Thanks Bharata!

> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> CC: Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>
> CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> CC: Ingo Molnar <mingo@elte.hu>
> ---
>  kernel/sched_fair.c |   26 +++++++++-----------------
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> --- a/kernel/sched_fair.c
> +++ b/kernel/sched_fair.c
> @@ -586,11 +586,12 @@ account_entity_enqueue(struct cfs_rq *cf
>  	update_load_add(&cfs_rq->load, se->load.weight);
>  	if (!parent_entity(se))
>  		inc_cpu_load(rq_of(cfs_rq), se->load.weight);
> -	if (entity_is_task(se))
> +	if (entity_is_task(se)) {
>  		add_cfs_task_weight(cfs_rq, se->load.weight);
> +		list_add(&se->group_node, &cfs_rq->tasks);
> +	}
>  	cfs_rq->nr_running++;
>  	se->on_rq = 1;
> -	list_add(&se->group_node, &cfs_rq->tasks);
>  }
>  
>  static void
> @@ -599,11 +600,12 @@ account_entity_dequeue(struct cfs_rq *cf
>  	update_load_sub(&cfs_rq->load, se->load.weight);
>  	if (!parent_entity(se))
>  		dec_cpu_load(rq_of(cfs_rq), se->load.weight);
> -	if (entity_is_task(se))
> +	if (entity_is_task(se)) {
>  		add_cfs_task_weight(cfs_rq, -se->load.weight);
> +		list_del_init(&se->group_node);
> +	}
>  	cfs_rq->nr_running--;
>  	se->on_rq = 0;
> -	list_del_init(&se->group_node);
>  }
>  
>  static void enqueue_sleeper(struct cfs_rq *cfs_rq, struct sched_entity *se)
> @@ -1445,19 +1447,9 @@ __load_balance_iterator(struct cfs_rq *c
>  	if (next == &cfs_rq->tasks)
>  		return NULL;
>  
> -	/* Skip over entities that are not tasks */
> -	do {
> -		se = list_entry(next, struct sched_entity, group_node);
> -		next = next->next;
> -	} while (next != &cfs_rq->tasks && !entity_is_task(se));
> -
> -	if (next == &cfs_rq->tasks)
> -		return NULL;
> -
> -	cfs_rq->balance_iterator = next;
> -
> -	if (entity_is_task(se))
> -		p = task_of(se);
> +	se = list_entry(next, struct sched_entity, group_node);
> +	p = task_of(se);
> +	cfs_rq->balance_iterator = next->next;
>  
>  	return p;
>  }
> 
> Regards,
> Bharata.