| 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
| ||
|
Message-Id: <203221324412916@web157.yandex.ru>
Date: Wed, 21 Dec 2011 00:28:36 +0400
From: Tkhai Kirill <tkhai@...dex.ru>
To: Oleg Nesterov <oleg@...hat.com>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
Peter Zijlstra <peterz@...radead.org>,
Gregory Haskins <ghaskins@...ell.com>
Subject: Re: [PATCH]sched_rt.c: Avoid unnecessary dequeue and enqueue of pushable tasks in set_cpus_allowed_rt()
20.12.2011, 21:44, "Oleg Nesterov" <oleg@...hat.com>:
> On 12/02, Kirill Tkhai wrote:
>
>> Migration status depends on a difference of weight from 0 and 1. If
>> weight > 1 (<= 1) and old weight <= 1 (> 1) then task becomes pushable
>> (not pushable). We are not insterested in exact values of it, is it 3 or
>> 4, for example.
>>
>> Now if we are changing affinity from a set of 3 cpus to a set of 4, the
>> task will be dequeued and enqueued sequentially without important
>> difference in comparison with initial state. The only difference is in
>> internal representation of plist queue of pushable tasks and the fact
>> that the task may won't be the first in a sequence of the same priority
>> tasks. But it seems to me it gives nothing.
>
> Looks reasonable, although I can't say I really understand this code.
> Add Gregory.
>
>> Signed-off-by: Tkhai Kirill <tkhai@...dex.ru>
>>
>> --- kernel/sched_rt.c.orig 2011-12-02 00:29:11.970243145 +0400
>> +++ kernel/sched_rt.c 2011-12-02 00:37:43.622846606 +0400
>
> please use -p1
>
Sorry, this time I'm sending "git diffed" output.
>> @@ -1572,43 +1572,37 @@ static void set_cpus_allowed_rt(struct t
>> const struct cpumask *new_mask)
>> {
>> int weight = cpumask_weight(new_mask);
>> + struct rq *rq;
>>
>> BUG_ON(!rt_task(p));
>>
>> /*
>> - * Update the migration status of the RQ if we have an RT task
>> - * which is running AND changing its weight value.
>> + * Just exit if it's not necessary to change migration status
>> */
>> - if (p->on_rq && (weight != p->rt.nr_cpus_allowed)) {
>> - struct rq *rq = task_rq(p);
>> + if ((p->rt.nr_cpus_allowed <= 1 && weight <= 1)
>> + || (p->rt.nr_cpus_allowed > 1 && weight > 1))
>> + return;
>
> Subjective, but may be
>
> if ((p->rt.nr_cpus_allowed > 1) != (weight > 1))
> return;
>
> looks more understandable?
Yes, thanks.
---
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 3640ebb..4467f4d 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1774,43 +1774,36 @@ static void set_cpus_allowed_rt(struct task_struct *p,
const struct cpumask *new_mask)
{
int weight = cpumask_weight(new_mask);
+ struct rq *rq;
BUG_ON(!rt_task(p));
/*
- * Update the migration status of the RQ if we have an RT task
- * which is running AND changing its weight value.
+ * Just exit if it's not necessary to change migration status
*/
- if (p->on_rq && (weight != p->rt.nr_cpus_allowed)) {
- struct rq *rq = task_rq(p);
-
- if (!task_current(rq, p)) {
- /*
- * Make sure we dequeue this task from the pushable list
- * before going further. It will either remain off of
- * the list because we are no longer pushable, or it
- * will be requeued.
- */
- if (p->rt.nr_cpus_allowed > 1)
- dequeue_pushable_task(rq, p);
-
- /*
- * Requeue if our weight is changing and still > 1
- */
- if (weight > 1)
- enqueue_pushable_task(rq, p);
+ if ((p->rt.nr_cpus_allowed > 1) != (weight > 1))
+ return;
- }
+ if (!p->on_rq)
+ return;
- if ((p->rt.nr_cpus_allowed <= 1) && (weight > 1)) {
- rq->rt.rt_nr_migratory++;
- } else if ((p->rt.nr_cpus_allowed > 1) && (weight <= 1)) {
- BUG_ON(!rq->rt.rt_nr_migratory);
- rq->rt.rt_nr_migratory--;
- }
+ rq = task_rq(p);
- update_rt_migration(&rq->rt);
+ /*
+ * Several cpus were allowed but now it's not so OR vice versa
+ */
+ if (weight <= 1) {
+ if (!task_current(rq, p))
+ dequeue_pushable_task(rq, p);
+ BUG_ON(!rq->rt.rt_nr_migratory);
+ rq->rt.rt_nr_migratory--;
+ } else {
+ if (!task_current(rq, p))
+ enqueue_pushable_task(rq, p);
+ rq->rt.rt_nr_migratory++;
}
+
+ update_rt_migration(&rq->rt);
}
/* Assumes rq->lock is held */
--
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/
Powered by blists - more mailing lists