linux-kernel - Re: check_preempt_tick() check vruntime mistake?

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 11 Dec 2009 08:25:26 +0100
From:	Mike Galbraith <efault@....de>
To:	XingChao Wang <wxc200@...il.com>
Cc:	kernel list <linux-kernel@...r.kernel.org>, mingo@...e.hu,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: check_preempt_tick() check vruntime mistake?

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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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