| 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
| ||
|
Date: Mon, 09 Mar 2009 20:46:27 +1030
From: David Newall <davidn@...idnewall.com>
To: Ingo Molnar <mingo@...e.hu>
CC: Mike Galbraith <efault@....de>,
Balazs Scheidler <bazsi@...abit.hu>,
linux-kernel@...r.kernel.org,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Willy Tarreau <w@....eu>
Subject: Re: [patch] Re: scheduler oddity [bug?]
Ingo Molnar wrote:
> * Mike Galbraith <efault@....de> wrote:
>
...
>> OK, I've not seen any problem indications yet, so find patchlet below.
>>
>> However! Balazs has stated that this problem is _not_ present in .git,
>> and that..
>>
>> commit 38736f475071b80b66be28af7b44c854073699cc
>> Author: Gautham R Shenoy <ego@...ibm.com>
>> Date: Sat Sep 6 14:50:23 2008 +0530
>>
>> ..is what fixed it. Willy Tarreau verified this as being the case on
>> his HW as well. It is present in .git with my HW.
>>
>> I see it as a problem, but it's your call. Dunno if I'd apply it or
>> hold back, given these conflicting reports.
>>
>
> I think we still want it - as the purpose of the overlap metric
> is to measure reality. If preemption causes overlap in execution
> we should not ignore that.
>
I'm sure it's wrong. The only call to dequeue with a non-zero sleep
value is in deactivate_task. All the rest have zero sleep. The section
of code identified by Mike in his patchlet should be moved for purpose
of clarity. It also hilights the symmetry between queue_task and
dequeue_task:
--- sched.c 2009-02-21 09:09:34.000000000 +1030
+++ sched.c.dn 2009-03-09 20:13:51.000000000 +1030
@@ -1647,12 +1647,6 @@
static void dequeue_task(struct rq *rq, struct task_struct *p, int sleep)
{
- if (sleep && p->se.last_wakeup) {
- update_avg(&p->se.avg_overlap,
- p->se.sum_exec_runtime - p->se.last_wakeup);
- p->se.last_wakeup = 0;
- }
-
sched_info_dequeued(p);
p->sched_class->dequeue_task(rq, p, sleep);
p->se.on_rq = 0;
@@ -1724,6 +1718,12 @@
if (task_contributes_to_load(p))
rq->nr_uninterruptible++;
+ if (sleep && p->se.last_wakeup) {
+ update_avg(&p->se.avg_overlap,
+ p->se.sum_exec_runtime - p->se.last_wakeup);
+ p->se.last_wakeup = 0;
+ }
+
dequeue_task(rq, p, sleep);
dec_nr_running(rq);
}
Having done that, it makes sense to entirely remove dequeue_task 's
sleep parameter, and replicate all three lines in deactivate_task:
--- sched.c.dn 2009-03-09 20:41:13.000000000 +1030
+++ sched.c.dn2 2009-03-09 20:41:30.000000000 +1030
@@ -1645,10 +1645,10 @@
p->se.on_rq = 1;
}
-static void dequeue_task(struct rq *rq, struct task_struct *p, int sleep)
+static void dequeue_task(struct rq *rq, struct task_struct *p)
{
sched_info_dequeued(p);
- p->sched_class->dequeue_task(rq, p, sleep);
+ p->sched_class->dequeue_task(rq, p, 0);
p->se.on_rq = 0;
}
@@ -1724,7 +1724,11 @@
p->se.last_wakeup = 0;
}
- dequeue_task(rq, p, sleep);
+ /*dequeue_task(rq, p, sleep);*/
+ sched_info_dequeued(p);
+ p->sched_class->dequeue_task(rq, p, sleep);
+ p->se.on_rq = 0;
+
dec_nr_running(rq);
}
@@ -4848,7 +4852,7 @@
on_rq = p->se.on_rq;
running = task_current(rq, p);
if (on_rq)
- dequeue_task(rq, p, 0);
+ dequeue_task(rq, p);
if (running)
p->sched_class->put_prev_task(rq, p);
@@ -4897,7 +4901,7 @@
}
on_rq = p->se.on_rq;
if (on_rq)
- dequeue_task(rq, p, 0);
+ dequeue_task(rq, p);
p->static_prio = NICE_TO_PRIO(nice);
set_load_weight(p);
@@ -8637,7 +8641,7 @@
on_rq = tsk->se.on_rq;
if (on_rq)
- dequeue_task(rq, tsk, 0);
+ dequeue_task(rq, tsk);
if (unlikely(running))
tsk->sched_class->put_prev_task(rq, tsk);
--
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