lkml.org 
[lkml]   [2005]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] kernel <linux-2.6.11.10> kernel/sched.c
    /*===== ISSUE ====*/
    My second version of patch has a defect.

    + if (unlikely(old_prio != next->prio)) {
    + dequeue_task(next, array); --> ### dequeue should against
    old_prio, NOT next->prio ###
    + enqueue_task(next, array);
    + }

    unforunately, dequeue_task does not accept the third parameter to make
    adjustment. Personally, I feel it's good to add extra function as my
    first version of patch to combine dequeue and enqueue together.
    Reasons as following:
    1) adding the third parameter to dequeue_task() would cause other
    places' code change;
    2) for schedule functions, performance is the first consideration.
    Notice both dequeue_task() and enqueue_task() are NOT inline.
    Combining those two in one saves one function call overhead;


    /* ===== NEW PATCH ===== */
    The new patch, see below, adds new function change_queue_task() to
    dequeue from "old_prio queue" and enqueue the "next task" to
    "next->prio queue".

    The patch also inlines requeue_task().

    The patch has been tested with 2.6.11.10, looks good. -For somehow,
    2.6.12-rc4 is still not stable on my machine (Fedora 3).


    /* ===== [PATCH 2.6.11.?] kernel/sched.c =====*/
    --- linux-2.6.12-rc4.orig/kernel/sched.c 2005-05-06 22:20:31.000000000 -0700
    +++ linux12/kernel/sched.c 2005-05-21 16:19:11.000000000 -0700
    @@ -556,11 +556,23 @@
    p->array = array;
    }

    +static void change_queue_task(struct task_struct *p, prio_array_t
    *array, int old_prio)
    +{
    + list_del(&p->run_list);
    + if (list_empty(array->queue + old_prio))
    + __clear_bit(old_prio, array->bitmap);
    +
    + sched_info_queued(p);
    + list_add_tail(&p->run_list, array->queue + p->prio);
    + __set_bit(p->prio, array->bitmap);
    + p->array = array;
    +}
    +
    /*
    * Put task to the end of the run list without the overhead of dequeue
    * followed by enqueue.
    */
    -static void requeue_task(struct task_struct *p, prio_array_t *array)
    +static inline void requeue_task(struct task_struct *p, prio_array_t *array)
    {
    list_move_tail(&p->run_list, array->queue + p->prio);
    }
    @@ -2613,7 +2625,7 @@
    struct list_head *queue;
    unsigned long long now;
    unsigned long run_time;
    - int cpu, idx;
    + int cpu, idx, old_prio;

    /*
    * Test if we are atomic. Since do_exit() needs to call into
    @@ -2735,9 +2747,14 @@
    delta = delta * (ON_RUNQUEUE_WEIGHT * 128 / 100) / 128;

    array = next->array;
    - dequeue_task(next, array);
    + old_prio = next->prio;
    +
    recalc_task_prio(next, next->timestamp + delta);
    - enqueue_task(next, array);
    +
    + if (unlikely(old_prio != next->prio))
    + change_queue_task(next, array, old_prio);
    + else
    + requeue_task(next, array);
    }
    next->activated = 0;
    switch_tasks:
    /* ===== PATCH END ===== */
    Thanks,
    -chen

    On 5/20/05, Ingo Molnar <mingo@elte.hu> wrote:
    >
    > * Con Kolivas <kernel@kolivas.org> wrote:
    >
    > > On Fri, 20 May 2005 19:49, Ingo Molnar wrote:
    > > > * chen Shang <shangcs@gmail.com> wrote:
    > > > > I minimized my patch and against to 2.6.12-rc4 this time, see below.
    > > >
    > > > looks good - i've done some small style/whitespace cleanups and renamed
    > > > prio to old_prio, patch against -rc4 below.
    > >
    > > We should inline requeue_task as well.
    >
    > yeah.
    >
    > Acked-by: Ingo Molnar <mingo@elte.hu>
    >
    > Ingo
    >
    -
    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/

    \
     
     \ /
      Last update: 2005-05-22 06:44    [W:0.025 / U:1.468 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site