Date:   Wed, 8 Mar 2023 22:33:36 +0800
From:   Chen Yu <yu.c.chen@intel.com>
To:     Zhang Qiao <zhangqiao22@huawei.com>
CC:     <linux-kernel@vger.kernel.org>, <mingo@redhat.com>,
        <peterz@infradead.org>, <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>, <rkagan@amazon.de>
Subject: Re: [PATCH v2] sched/fair: sanitize vruntime of entity being migrated
Message-ID: <ZAicwDeZ9/cxU8Eo@chenyu5-mobl1>
References: <20230306132418.50389-1-zhangqiao22@huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230306132418.50389-1-zhangqiao22@huawei.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?hGiOEIhHN/WixhhgSs29kU6yIH0NEMRWQuXJKrYPiioFrdF9OgsOp1f6SPwl?=
 =?us-ascii?Q?t+XPydSeEW1lNHxo8QvQK9lf4UwEkWMyEZE4BBUgcyMPbX3xifu7ULMeOJ2j?=
 =?us-ascii?Q?XosJPQ99rVApG7Fo5AtmEa/cFSNfEJzrufu6ocxBaIP1NB1ggpsvtSBpHilU?=
 =?us-ascii?Q?ajUe3faZVMAKmeGniWUqOo/9734rA2iLj1GnpWH/SnXVA8W1gnT/Z1kLQLhw?=
 =?us-ascii?Q?YrSFxS953nTyBk0KExRuE9vIfMQdhl55iNs9ISO4rQhC40VIzPeBUl5t4KMK?=
 =?us-ascii?Q?xPTYbmai1i4kN8l3ONJdW5iKdI2wmqxqdp9wt5aDCnSlTRoEqC0IV1G/xRSR?=
 =?us-ascii?Q?bF+Q18HGQad2fCMlQ/IKJbahSQ89EUhQjZHjbu0EwIVDdRK+FBSOCQaPD3Gh?=
 =?us-ascii?Q?wPyoKK89BMTLakTlC8zqry7hAQ7NC3913+RrNAw2utx+vaqbI4EIxtKbn4vy?=
 =?us-ascii?Q?b3hVxi1TEUjq/rAVBTfS6d3x63lCnIse0VIwPcCCzse1KalWBpH91MWvazhc?=
 =?us-ascii?Q?DS8oJDHjLGyLB4VoFRIdKkWIWDYB8Nlm840bePdMdG7w/+auJbyVIOZyzLZ0?=
 =?us-ascii?Q?l59j3voBAzeODDgNu2TnSCOymVpQKs+1Dkc99GThvJSJkLes3NpD7Tiz1o/H?=
 =?us-ascii?Q?SlbaGsug7wjc6QUZc8DhG5nNZR4RWZafKr6hIsqQHrOFBpfX6bRzQbkLLOSD?=
 =?us-ascii?Q?ZZL5HdZ6ecyjO2/ZIpB48/ol2heIed9rS83k8hRib7xVjS2AOefhBOv2tZsr?=
 =?us-ascii?Q?gdOPPFjp4AAhsln3TcsPTJ7L1eYhj9h67R8jmYmpDGlOTUWGmyZ+fWcd54va?=
 =?us-ascii?Q?vjmkEm1wF5Dr8y87+Zhy5VVz/x2if3KhE4CAqKgzUDr9vIH6B/NcfWq7cuBF?=
 =?us-ascii?Q?XNjSGoH1xRFjqC8ZIymMsuvPPhZg1xdrMI7uD+yg6uPmj9az7PsEeTzNtzsb?=
 =?us-ascii?Q?PKC+IjWnN51GEhxZpQPoLesCnzx8RrWhGXH13HN16uAKehUTsfzduLC6T4fR?=
 =?us-ascii?Q?7CjT5b0be7FNEO13apyWFmbB8jzBQrJLjgG3MmC7Vs25H6LIBvSHX3AB4oOd?=
 =?us-ascii?Q?z5Z2fzZs+xTO2fPhznxkDzKwcc2SUKXhodhNVjdrD2GNwyrIEgPOcgjnoTcF?=
 =?us-ascii?Q?OjlpkMUsOECVq3Pmzst3Cn7zi56qbKX89qilIqX+nSomKi7Ys2Ljkl+dH0L7?=
 =?us-ascii?Q?Y83uo25PB8zejd9boIIP5h3w522ZzWYQlqJuiGLSBu/1gWjLJyfoMsif9XGK?=
 =?us-ascii?Q?fAZuThx/bXQvBZyqf1JG3idPeF3Roo/mJroU/fK6CLxYE4POn//rqL/T+4/G?=
 =?us-ascii?Q?YbYRvSaZOu7ARQ3q/HG2Tgwq+fGqgXPlJBvIKOXg8sOW/0O8CEB2pjBwRSbB?=
 =?us-ascii?Q?zJudJ5S7vHoIMa63VtQJsq9DeSP8mQPjzVjxdd6Dl2dF0EDVhGU1XbmTTmIq?=
 =?us-ascii?Q?77TldVr0xZuQ0pAPAxav2Ju98iwjPmvULo2TTYppOCT6jxdc0D1nlJGMquhX?=
 =?us-ascii?Q?7DcTesSROR8y8SbdYGPHTVqTrnv64DCu1sanvc7/VHjBcmdYDq+BvN0i0URh?=
 =?us-ascii?Q?0zMW6cDV2PfBXn2Uqp882T2qTs3Tj7UWNwQ13OMD?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 216f701d-08a6-485f-f7d0-08db1fe225de
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6206.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2023 14:33:56.5402
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ussmvGuugxj8Bn3UkH4HVJMiL3o6AMEIaim7e/6tg8S4Q0PrS0vlx/bSB/EOPn4aKxFmKYXIj84zMlFSLCjI3A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6977
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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