[PATCH] small optimization to update_curr_rt - Kernel

This is a discussion on [PATCH] small optimization to update_curr_rt - Kernel ; A very minor improvement, but might it be better to check sched_rt_runtime(rt_rq) before taking the rt_runtime_lock? Signed-off-by: Dimitri Sivanich -- kernel/sched_rt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: linux/kernel/sched_rt.c ================================================== ================= --- linux.orig/kernel/sched_rt.c 2008-10-22 16:10:03.000000000 -0500 ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [PATCH] small optimization to update_curr_rt

  1. [PATCH] small optimization to update_curr_rt

    A very minor improvement, but might it be better to check sched_rt_runtime(rt_rq)
    before taking the rt_runtime_lock?

    Signed-off-by: Dimitri Sivanich

    --

    kernel/sched_rt.c | 4 ++--
    1 file changed, 2 insertions(+), 2 deletions(-)

    Index: linux/kernel/sched_rt.c
    ================================================== =================
    --- linux.orig/kernel/sched_rt.c 2008-10-22 16:10:03.000000000 -0500
    +++ linux/kernel/sched_rt.c 2008-10-31 07:57:19.000000000 -0500
    @@ -537,13 +537,13 @@ static void update_curr_rt(struct rq *rq
    for_each_sched_rt_entity(rt_se) {
    rt_rq = rt_rq_of_se(rt_se);

    - spin_lock(&rt_rq->rt_runtime_lock);
    if (sched_rt_runtime(rt_rq) != RUNTIME_INF) {
    + spin_lock(&rt_rq->rt_runtime_lock);
    rt_rq->rt_time += delta_exec;
    if (sched_rt_runtime_exceeded(rt_rq))
    resched_task(curr);
    + spin_unlock(&rt_rq->rt_runtime_lock);
    }
    - spin_unlock(&rt_rq->rt_runtime_lock);
    }
    }

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

  2. Re: [PATCH] small optimization to update_curr_rt

    On Fri, Oct 31, 2008 at 6:03 AM, Dimitri Sivanich wrote:
    > A very minor improvement, but might it be better to check sched_rt_runtime(rt_rq)
    > before taking the rt_runtime_lock?


    Is it possible that the attribute sched_rt_runtime is checking could
    change by the time it acquires the lock? If not, should be fine, I
    think.

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

  3. Re: [PATCH] small optimization to update_curr_rt

    On Fri, Oct 31, 2008 at 06:10:13AM -0700, Steven Noonan wrote:
    > On Fri, Oct 31, 2008 at 6:03 AM, Dimitri Sivanich wrote:
    > > A very minor improvement, but might it be better to check sched_rt_runtime(rt_rq)
    > > before taking the rt_runtime_lock?

    >
    > Is it possible that the attribute sched_rt_runtime is checking could
    > change by the time it acquires the lock? If not, should be fine, I
    > think.
    >


    Steve,

    While it might be possible for it to change in that instant, I don't know if it matters.

    If the runtime value should change to RUNTIME_INF in that instant, it will be caught in sched_rt_runtime_exceeded(). If it changed from RUNTIME_INF to a lower value, I doubt it would matter much, as at most one more rt_rq value wouldn't be checked. Either way some rt_rq values would have been checked during the loop and some would not.
    --
    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/

  4. Re: [PATCH] small optimization to update_curr_rt

    On Fri, 2008-10-31 at 08:03 -0500, Dimitri Sivanich wrote:
    > A very minor improvement, but might it be better to check sched_rt_runtime(rt_rq)
    > before taking the rt_runtime_lock?


    Yes, I think its ok to do so.

    Like pointed out in the other thread, there are two races:

    - sched_rt_runtime() going to RUNTIME_INF, and that will be handled
    properly by sched_rt_runtime_exceeded()

    - sched_rt_runtime() going to !RUNTIME_INF, and here we can miss an
    accounting cycle, but I don't think that is something to worry too
    much about.

    Acked-by: Peter Zijlstra

    > Signed-off-by: Dimitri Sivanich
    >
    > --
    >
    > kernel/sched_rt.c | 4 ++--
    > 1 file changed, 2 insertions(+), 2 deletions(-)
    >
    > Index: linux/kernel/sched_rt.c
    > ================================================== =================
    > --- linux.orig/kernel/sched_rt.c 2008-10-22 16:10:03.000000000 -0500
    > +++ linux/kernel/sched_rt.c 2008-10-31 07:57:19.000000000 -0500
    > @@ -537,13 +537,13 @@ static void update_curr_rt(struct rq *rq
    > for_each_sched_rt_entity(rt_se) {
    > rt_rq = rt_rq_of_se(rt_se);
    >
    > - spin_lock(&rt_rq->rt_runtime_lock);
    > if (sched_rt_runtime(rt_rq) != RUNTIME_INF) {
    > + spin_lock(&rt_rq->rt_runtime_lock);
    > rt_rq->rt_time += delta_exec;
    > if (sched_rt_runtime_exceeded(rt_rq))
    > resched_task(curr);
    > + spin_unlock(&rt_rq->rt_runtime_lock);
    > }
    > - spin_unlock(&rt_rq->rt_runtime_lock);
    > }
    > }
    >

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

  5. Re: [PATCH] small optimization to update_curr_rt


    * Peter Zijlstra wrote:

    > On Fri, 2008-10-31 at 08:03 -0500, Dimitri Sivanich wrote:
    > > A very minor improvement, but might it be better to check sched_rt_runtime(rt_rq)
    > > before taking the rt_runtime_lock?

    >
    > Yes, I think its ok to do so.
    >
    > Like pointed out in the other thread, there are two races:
    >
    > - sched_rt_runtime() going to RUNTIME_INF, and that will be handled
    > properly by sched_rt_runtime_exceeded()
    >
    > - sched_rt_runtime() going to !RUNTIME_INF, and here we can miss an
    > accounting cycle, but I don't think that is something to worry too
    > much about.
    >
    > Acked-by: Peter Zijlstra


    applied to tip/sched/rt, thanks guys!

    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/

+ Reply to Thread