Re: [sched/rt] Optimization of function pull_rt_task()

[Yong Zhang <yong.zhang0@gmail.com>]

On Wed, Apr 18, 2012 at 05:16:55PM -0400, Steven Rostedt wrote:
> On Wed, 2012-04-18 at 14:32 -0400, Steven Rostedt wrote:
> > On Mon, 2012-04-16 at 12:06 -0400, Steven Rostedt wrote:
> > > On Sun, 2012-04-15 at 23:45 +0400, Kirill Tkhai wrote:
> > > > The condition (src_rq->rt.rt_nr_running) is weak because it doesn't
> > > > consider the cases when src_rq has only processes bound to it (when
> > > > single cpu is allowed). It may be running kernel thread like
> > > > migration/x etc.
> > > > 
> > > > So it's better to use more stronger condition which is able to exclude
> > > > above conditions. The function has_pushable_tasks() complitely does
> > > > this. A task may be pullable for another cpu rq only if he is pushable
> > > > for his own queue.
> > > 
> > > I considered this before, and for some reason I never did the change.
> > > I'll have to think about it. It seems like this would be the obvious
> > > case, but I think there was something not so obvious that caused issues.
> > > But I don't remember what it was.
> > > 
> > > I'll have to rethink this again.
> > 
> > I can't find anything wrong with this change. Maybe things change, or I
> > was thinking of another change.
> > 
> > I'll apply it and start running my tests against it.
> 
> Not only does this seem to work fine, I took it one step further :-)

Hmm... throttle doesn't handle the pushable list, so we may find a
throttled task by pick_next_pushable_task().

Thanks,
Yong

> 
> Peter, do you see anything wrong with this patch?
> 
> -- Steve
> 
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 61e3086..b44fd1b 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1416,39 +1416,15 @@ static int pick_rt_task(struct rq *rq, struct task_struct *p, int cpu)
>  /* Return the second highest RT task, NULL otherwise */
>  static struct task_struct *pick_next_highest_task_rt(struct rq *rq, int cpu)
>  {
> -	struct task_struct *next = NULL;
> -	struct sched_rt_entity *rt_se;
> -	struct rt_prio_array *array;
> -	struct rt_rq *rt_rq;
> -	int idx;
> +	struct plist_head *head = &rq->rt.pushable_tasks;
> +	struct task_struct *next;
>  
> -	for_each_leaf_rt_rq(rt_rq, rq) {
> -		array = &rt_rq->active;
> -		idx = sched_find_first_bit(array->bitmap);
> -next_idx:
> -		if (idx >= MAX_RT_PRIO)
> -			continue;
> -		if (next && next->prio <= idx)
> -			continue;
> -		list_for_each_entry(rt_se, array->queue + idx, run_list) {
> -			struct task_struct *p;
> -
> -			if (!rt_entity_is_task(rt_se))
> -				continue;
> -
> -			p = rt_task_of(rt_se);
> -			if (pick_rt_task(rq, p, cpu)) {
> -				next = p;
> -				break;
> -			}
> -		}
> -		if (!next) {
> -			idx = find_next_bit(array->bitmap, MAX_RT_PRIO, idx+1);
> -			goto next_idx;
> -		}
> +	plist_for_each_entry(next, head, pushable_tasks) {
> +		if (pick_rt_task(rq, next, cpu))
> +			return next;
>  	}
>  
> -	return next;
> +	return NULL;
>  }
>  
>  static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Only stand for myself