lkml.org 
[lkml]   [2009]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] (latest tip) make dequeue_task less confusing

* David Newall <davidn@davidnewall.com> wrote:

> The dequeue_patch function in kernel/sched.c is complicated by
> including a sleep parameter. This parameter is always zero
> except in one instance. This patch clarifies the task of
> dequeue_patch by removing the sleep parameter and moving the
> code that handles non-zero sleep to that one place where it is
> needed.
>

> --- kernel/sched.c 2009-03-12 18:41:41.000000000 +1030
> +++ kernel/sched.c.dn 2009-03-12 18:45:18.000000000 +1030
> @@ -1791,21 +1791,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)
> {
> - if (sleep) {
> - if (p->se.last_wakeup) {
> - update_avg(&p->se.avg_overlap,
> - p->se.sum_exec_runtime - p->se.last_wakeup);
> - p->se.last_wakeup = 0;
> - } else {
> - update_avg(&p->se.avg_wakeup,
> - sysctl_sched_wakeup_granularity);
> - }
> - }
> -
> 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;
> }
>
> @@ -1875,7 +1864,22 @@
> if (task_contributes_to_load(p))
> rq->nr_uninterruptible++;
>
> - dequeue_task(rq, p, sleep);
> + if (sleep) {
> + if (p->se.last_wakeup) {
> + update_avg(&p->se.avg_overlap,
> + p->se.sum_exec_runtime - p->se.last_wakeup);
> + p->se.last_wakeup = 0;
> + } else {
> + update_avg(&p->se.avg_wakeup,
> + sysctl_sched_wakeup_granularity);
> + }
> + }
> +
> + /*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);
> }
>
> @@ -5323,7 +5327,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);
>
> @@ -5372,7 +5376,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);
> @@ -9189,7 +9193,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);

It would be cleaner to achieve this by introducing a
__dequeue_task() inline function that does not have the sleep
parameter and is a thin wrapper over
p->sched_class->dequeue_task, and keep dequeue_task() with the
sleep parameter - but update it to use __dequeue_task() and mark
it an inline function.

[ Btw., could you please submit future patches in the regular
-p1 format? See Documentation/SubmittingPatches. And please
Cc: me to future scheduler patches. Thanks! ]

Ingo


\
 
 \ /
  Last update: 2009-03-12 10:59    [W:0.065 / U:0.784 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site