check_preempt_tick() check vruntime mistake?

check_preempt_tick() check vruntime mistake?

[XingChao Wang]

Hi Ingo,peter,

When check_preempt_tick() selects next leftmost sched_entity,it
calculates delta vruntime of curr and leftmost entity, then compares
it with ideal_runtime. But ideal_runtime is real-time type, need
convert it to virtual-time ,right?

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 37087a7..9b903d4 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -840,7 +840,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct
sched_entity *curr)
                struct sched_entity *se = __pick_next_entity(cfs_rq);
                s64 delta = curr->vruntime - se->vruntime;

-               if (delta > ideal_runtime)
+               if (delta > calc_delta_fair(ideal_runtime, curr))
                        resched_task(rq_of(cfs_rq)->curr);
        }
 }

thanks
--wang xingchao

Re: check_preempt_tick() check vruntime mistake?

[Mike Galbraith]

On Fri, 2009-12-11 at 12:17 +0800, XingChao Wang wrote:
> Hi Ingo,peter,
> 
> When check_preempt_tick() selects next leftmost sched_entity,it
> calculates delta vruntime of curr and leftmost entity, then compares
> it with ideal_runtime. But ideal_runtime is real-time type, need
> convert it to virtual-time ,right?

Why?  The scheduler converges vruntimes to within min_granularity,
that's it's mission.  What this test is trying to say is that if the
awakened task's lag has grown to be more than your quantum while we
waited for the tick to come along, you need to get out of it's way so it
can catch up.

Now, there are a couple ~problems in that in it's current form, it
offers protection to SCHED_BATCH and SCHED_IDLE tasks, and the test..

if (delta_exec < sysctl_sched_min_granularity)
	return;

..is... not exactly optimal for light weight tasks, but the lighter you
get, the more likely you'd already have been preempted...

I have a patch which excludes these, and computes min runtime based on
differential in task weights.  These are the only things I see ~wrong.
ideal_runtime is weighted, and is our "fair unit of measure", so using
it to measure acceptable lag distance is correct AFAIKS.

> diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
> index 37087a7..9b903d4 100644
> --- a/kernel/sched_fair.c
> +++ b/kernel/sched_fair.c
> @@ -840,7 +840,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct
> sched_entity *curr)
>                 struct sched_entity *se = __pick_next_entity(cfs_rq);
>                 s64 delta = curr->vruntime - se->vruntime;
> 
> -               if (delta > ideal_runtime)
> +               if (delta > calc_delta_fair(ideal_runtime, curr))
>                         resched_task(rq_of(cfs_rq)->curr);
>         }
>  }
> 
> thanks
> --wang xingchao
> --
> 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/