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]

Message-ID: <CAB8ipk-Z=CkvbVczCpmCgmymKqWQfuKNF7xm51s_03obAen_Ew@mail.gmail.com>
Date: Mon, 5 Feb 2024 14:36:35 +0800
From: Xuewen Yan <xuewen.yan94@...il.com>
To: Xuewen Yan <xuewen.yan@...soc.com>
Cc: peterz@...radead.org, 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, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched/eevdf: Prevent vlag from exceeding the limit value

Hi all,

On Tue, Jan 30, 2024 at 4:07 PM Xuewen Yan <xuewen.yan@...soc.com> 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.

Could anyone make some comments?

In my opinion, even if it may be ignored when reweighting, it is
better to solve the problem when forking.
Maybe we could change some settings in set_next_entity() where vlag is
equal to deadline?


Thanks!
--
BR
xuewen

>
> 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);
> +}
> +
>  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);
> @@ -5177,6 +5188,16 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>
>                 lag = se->vlag;
>
> +               /* When fork, the child would inherit parent's vlag,
> +                * but parent's vlag may be euqual to it's deadline.
> +                */
> +               if (flag & ENQUEUE_INITIAL) {
> +                       if (curr)
> +                               lag = vruntime - curr->vruntime;
> +
> +                       lag = limit_entity_lag(se, lag);
> +                       se->vlag = lag;
> +               }
>                 /*
>                  * If we want to place a task and preserve lag, we have to
>                  * consider the effect of the new entity on the weighted
> @@ -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
>