linux-kernel - Re: [PATCH] sched/eevdf: Prevent vlag from exceeding the limit value

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] [thread-next>] [day] [month] [year] [list]

Date: Mon, 8 Apr 2024 14:06:04 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Xuewen Yan <xuewen.yan@...soc.com>
Cc: mingo@...hat.com, juri.lelli@...hat.com, vincent.guittot@...aro.org,
	dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
	mgorman@...e.de, bristot@...hat.com, vschneid@...hat.com,
	wuyun.abel@...edance.com, ke.wang@...soc.com,
	xuewen.yan94@...il.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched/eevdf: Prevent vlag from exceeding the limit value

On Tue, Jan 30, 2024 at 04:06:43PM +0800, Xuewen Yan wrote:
> There are some scenarios here that will cause vlag to
> exceed eevdf's limit.
> 
> 1. In set_next_entity, it will stash a copy of deadline
> at the point of pick in vlag, and if the task fork child,
> the child's vlag would inherit parent's vlag, as a result,
> the child's vlag is equal to the deadline. And then,
> when place_entity, because the se's vlag is wrong, it would
> cause vruntime and deadline update error.

But __sched_fork() clears it? Or am I missing something subtle?

> 2. When reweight_entity, the vlag would be recalculated,
> because the new weights are uncertain, and it may cause
> the vlag to exceed eevdf's limit.
> 
> In order to ensure that vlag does not exceed the limit,
> separate the calculation of limit from the update_entity_lag
> and create a new func limit_entity_lag. When the vlag is
> updated, use this func to prevent vlag from exceeding the limit.
> 
> And add check whether the se is forked in place_entity, and calc
> the curr's vlag to update the se's vlag.
> 
> Signed-off-by: Xuewen Yan <xuewen.yan@...soc.com>
> ---
>  kernel/sched/fair.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 533547e3c90a..3fc99b4b8b41 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -696,15 +696,22 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq)
>   *
>   * XXX could add max_slice to the augmented data to track this.
>   */
> +static s64 limit_entity_lag(struct sched_entity *se, s64 lag)
> +{
> +	s64 limit;
> +
> +	limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se);
> +	return clamp(lag, -limit, limit);
> +}

I used to run with a trace_printk() in there to check how often this
trips, can you readily trigger these sites, or do you have to work hard
at it?


>  static void update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  {
> -	s64 lag, limit;
> +	s64 lag;
>  
>  	SCHED_WARN_ON(!se->on_rq);
>  	lag = avg_vruntime(cfs_rq) - se->vruntime;
>  
> -	limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se);
> -	se->vlag = clamp(lag, -limit, limit);
> +	se->vlag = limit_entity_lag(se, lag);
>  }
>  
>  /*
> @@ -3757,6 +3764,7 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  	if (avruntime != se->vruntime) {
>  		vlag = (s64)(avruntime - se->vruntime);
>  		vlag = div_s64(vlag * old_weight, weight);
> +		vlag = limit_entity_lag(se, vlag);
>  		se->vruntime = avruntime - vlag;
>  	}
>  
> @@ -3804,6 +3812,9 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  
>  	update_load_set(&se->load, weight);
>  
> +	if (!se->on_rq)
> +		se-vlag = limit_entity_lag(se, se->vlag);
> +
>  #ifdef CONFIG_SMP
>  	do {
>  		u32 divider = get_pelt_divider(&se->avg);

> @@ -5237,6 +5258,7 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  		if (WARN_ON_ONCE(!load))
>  			load = 1;
>  		lag = div_s64(lag, load);
> +		lag = limit_entity_lag(se, lag);
>  	}
>  
>  	se->vruntime = vruntime - lag;
> -- 
> 2.25.1
>