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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1308243122.9218.143.camel@gandalf.stny.rr.com>
Date:	Thu, 16 Jun 2011 12:52:02 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Hillf Danton <dhillf@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Mike Galbraith <efault@....de>,
	Yong Zhang <yong.zhang0@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] sched: find all lowest run-queues for RT task

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@...il.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;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/