Received-SPF: pass (google.com: domain of linux-kernel+bounces-52080-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240130080643.1828-1-xuewen.yan@unisoc.com>
In-Reply-To: <20240130080643.1828-1-xuewen.yan@unisoc.com>
From: Xuewen Yan <xuewen.yan94@gmail.com>
Date: Mon, 5 Feb 2024 14:36:35 +0800
Message-ID: <CAB8ipk-Z=CkvbVczCpmCgmymKqWQfuKNF7xm51s_03obAen_Ew@mail.gmail.com>
Subject: Re: [PATCH] sched/eevdf: Prevent vlag from exceeding the limit value
To: Xuewen Yan <xuewen.yan@unisoc.com>
Cc: peterz@infradead.org, mingo@redhat.com, juri.lelli@redhat.com, 
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com, rostedt@goodmis.org, 
	bsegall@google.com, mgorman@suse.de, bristot@redhat.com, vschneid@redhat.com, 
	wuyun.abel@bytedance.com, ke.wang@unisoc.com, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi all,

On Tue, Jan 30, 2024 at 4:07=E2=80=AFPM Xuewen Yan <xuewen.yan@unisoc.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@unisoc.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 =3D 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 =3D avg_vruntime(cfs_rq) - se->vruntime;
>
> -       limit =3D calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se)=
;
> -       se->vlag =3D clamp(lag, -limit, limit);
> +       se->vlag =3D limit_entity_lag(se, lag);
>  }
>
>  /*
> @@ -3757,6 +3764,7 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, s=
truct sched_entity *se,
>         if (avruntime !=3D se->vruntime) {
>                 vlag =3D (s64)(avruntime - se->vruntime);
>                 vlag =3D div_s64(vlag * old_weight, weight);
> +               vlag =3D limit_entity_lag(se, vlag);
>                 se->vruntime =3D 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 =3D limit_entity_lag(se, se->vlag);
> +
>  #ifdef CONFIG_SMP
>         do {
>                 u32 divider =3D get_pelt_divider(&se->avg);
> @@ -5177,6 +5188,16 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_e=
ntity *se, int flags)
>
>                 lag =3D 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 =3D vruntime - curr->vruntime;
> +
> +                       lag =3D limit_entity_lag(se, lag);
> +                       se->vlag =3D lag;
> +               }
>                 /*
>                  * If we want to place a task and preserve lag, we have t=
o
>                  * consider the effect of the new entity on the weighted
> @@ -5237,6 +5258,7 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_en=
tity *se, int flags)
>                 if (WARN_ON_ONCE(!load))
>                         load =3D 1;
>                 lag =3D div_s64(lag, load);
> +               lag =3D limit_entity_lag(se, lag);
>         }
>
>         se->vruntime =3D vruntime - lag;
> --
> 2.25.1
>