Re: [PATCH] sched: find all lowest run-queues for RT task

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 2011-06-03 at 22:11 +0800, Hillf Danton wrote:
> When finding the lowest run-queue for a given RT task, as seen in cases such as
> select_task_rq_rt() and find_lock_lowest_rq(), RQs with no RT tasks are also
> eligible for both RR and FIFO RT tasks.
> 
> Minor change is added, when initializing cpupri, to record non-RT RQs which are
> upated automatically, due to the cool design and implementation of cpupri, when
> RT RQs are set and unset. Then they could be included in the selected CPUs by
> cpupri_find().
> 
> A call for cpupri_set() is also added when a RQ has no RT task to run.

I just analyzed the code, and only the first part of the patch seems OK.


> Signed-off-by: Hillf Danton <dhillf@gmail.com>
> ---
>  kernel/sched_cpupri.c |    4 +++-
>  kernel/sched_rt.c     |    4 ++++
>  2 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/sched_cpupri.c b/kernel/sched_cpupri.c
> index 2722dc1..d4c0b46 100644
> --- a/kernel/sched_cpupri.c
> +++ b/kernel/sched_cpupri.c
> @@ -181,8 +181,10 @@ int cpupri_init(struct cpupri *cp)
>  			goto cleanup;
>  	}
> 
> -	for_each_possible_cpu(i)
> +	for_each_possible_cpu(i) {
>  		cp->cpu_to_pri[i] = CPUPRI_INVALID;
> +		cpupri_set(cp, i, MAX_RT_PRIO);
> +	}

This is OK, and I may add it. It's just an initialization, and because
things like the migration thread run on all CPUs, the first time an RT
task is run on a CPU and dequeued, it will have the cpupri_set() to
MX_RT_PRIO anyway. But for clean up sake, it may be good to initialize
it to a valid mask and not be ZERO.


>  	return 0;
> 
>  cleanup:
> diff --git a/kernel/sched_rt.c b/kernel/sched_rt.c
> index 88725c9..4dd6e45 100644
> --- a/kernel/sched_rt.c
> +++ b/kernel/sched_rt.c
> @@ -1158,6 +1158,10 @@ static struct task_struct
> *pick_next_task_rt(struct rq *rq)
>  	 * lock again later if there is no need to push
>  	 */
>  	rq->post_schedule = has_pushable_tasks(rq);
> +
> +	/* this CPU is no longer RT if no task picked to feed it */
> +	if (! p)
> +		cpupri_set(&rq->rd->cpupri, rq->cpu, MAX_RT_PRIO);

Now this is where things are not needed, because when we dequeue the
last RT task, this is already done by the dequeue code.

-- Steve


>  #endif
> 
>  	return p;