From: Kirill Tkhai <tkhai@yandex.ru>
To: Yong Zhang <yong.zhang0@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>
In-Reply-To: <20120419085440.GC3963@zhy>
References: <1334519122.8698.3.camel@hp>
 <1334592379.28106.4.camel@gandalf.stny.rr.com>
 <1334773976.28106.49.camel@gandalf.stny.rr.com>
 <1334783815.28106.56.camel@gandalf.stny.rr.com> <20120419085440.GC3963@zhy>
Subject: Re: [sched/rt] Optimization of function pull_rt_task()
MIME-Version: 1.0
Message-Id: <69791338569116@web3f.yandex.ru>
Date: Fri, 01 Jun 2012 20:45:16 +0400
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=koi8-r
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3733
Lines: 109


19.04.2012, 12:54, "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

I don't complitelly understand throttle logic.

Is the source patch not-appliable the same reason?

Kirill

>
>> ?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
--
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/