Re: [PATCH v2] sched/fair: sanitize vruntime of entity being migrated

[Chen Yu <yu.c.chen@intel.com>]

On 2023-03-06 at 21:24:18 +0800, Zhang Qiao wrote:
> Commit 829c1651e9c4 ("sched/fair: sanitize vruntime of
> entity being placed") fix an overflowing bug, but ignore
> a case that se->exec_start is reset after a migration.
> 
> For fixing this case, we reset the vruntime of a long
> sleeping task in migrate_task_rq_fair().
> 
> Fixes: 829c1651e9c4 ("sched/fair: sanitize vruntime of entity being placed")
> Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
> Signed-off-by: Zhang Qiao <zhangqiao22@huawei.com>
> ---
> 
> v1 -> v2:
> - fix some typos and update comments
> - reformat the patch
> 
> ---
>  kernel/sched/fair.c | 76 ++++++++++++++++++++++++++++++++-------------
>  1 file changed, 55 insertions(+), 21 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 7a1b1f855b96..74c9918ffe76 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4648,11 +4648,45 @@ static void check_spread(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  #endif
>  }
>  
> +static inline bool entity_is_long_sleep(struct sched_entity *se)
> +{
> +	struct cfs_rq *cfs_rq;
> +	u64 sleep_time;
> +
> +	if (se->exec_start == 0)
> +		return false;
> +
Just 2 cents, can we add some comments for above to
mention that, because the se is a migrated one, and we
have taken care of its vruntime during migration,
so no need to further massage the vruntime to cfs_rq->min_vruntime.

thanks,
Chenyu