Received-SPF: pass (google.com: domain of linux-kernel+bounces-153007-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Mon, 22 Apr 2024 16:47:31 +0800
From: Chen Yu <yu.c.chen@intel.com>
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>, <ke.wang@unisoc.com>,
	<xuewen.yan94@gmail.com>, <di.shen@unisoc.com>,
	<linux-kernel@vger.kernel.org>, Sergei Trofimovich <slyich@gmail.com>, "Breno
 Leitao" <leitao@debian.org>, Igor Raits <igor@gooddata.com>, Tim Chen
	<tim.c.chen@intel.com>, Yujie Liu <yujie.liu@intel.com>
Subject: Re: [PATCH v2] sched/eevdf: Prevent vlag from going out of bounds
 when reweight_eevdf
Message-ID: <ZiYkI4qn2nvdmktq@chenyu5-mobl2>
References: <20240422082238.5784-1-xuewen.yan@unisoc.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240422082238.5784-1-xuewen.yan@unisoc.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?LplJ7X9dHDX/SZ2Iw+oIS7FhVkJxy8vVa2NPYSEwvI+pMn4vld4VDORCWTGH?=
 =?us-ascii?Q?pNhoDyO+1hccpVMNoBjkST/k5hTQ18ImtQH2xs4rEwWM1pXPh4+mQJMsVpx8?=
 =?us-ascii?Q?8gQxcNoT16waKVsXCAH0P9s14AVnbFsnjB39ly0RyFs1+Gs3iVX+pGq7oiCg?=
 =?us-ascii?Q?Mbvl3ZbVbpp5D9A100EMZ6cOsgmGwc1tD7jijhMcv8wOpVJbcRamN8OUmAlz?=
 =?us-ascii?Q?qqtOOLZ4Im1Ttp69Fx+/1dsep2aHJbAFz4Z0F75HiUcyR3KeLfSu86Mfll7y?=
 =?us-ascii?Q?PC2irY8084n7KbUWkLOm0O9tjObX+oKgyqNoOtuSl4pCHkS4l/KEa8pbAaee?=
 =?us-ascii?Q?q8wjHlrrJP460jGpfU4R8mM60ABvqavrGqYn/2qfdidYqHG1QwT32N0kZOdi?=
 =?us-ascii?Q?C4jWKtQlFrQd7TTk0+q8/isYsJIUT/UT8gR5wo14qZIw7/DavqyaHveCrPKX?=
 =?us-ascii?Q?cMa7Eib2y1kqZbSC3uw+E5aAxkFZPy8+IWTHDDhYZvFSg9eeLiBsfanXBFhV?=
 =?us-ascii?Q?65x2rg0CwBmtDIMaY/KhEeQyAcxM6nchyq4QOs+UB06AY3Ztww0q9Ssxh0sO?=
 =?us-ascii?Q?fluvOIs9SlWYuz/+vzudYRUcE5ivcpPryqZTz/RZEtuCn2OJj0TTq04HUZaK?=
 =?us-ascii?Q?V92IcobRRomVzMpeQdYlE0MWCnZmrGMETVmaukVd0baHstXshCbSxbaa8Oj8?=
 =?us-ascii?Q?/Bu7HxOKPJyfJH+eruofknqQ5ogkDDyHYjiaNctEhMizW1aSqhbX5kyzIDnw?=
 =?us-ascii?Q?Rs1pc/FACLszsQq4Vh9dN35rYc7YCAuoyPgjK/7pL5ifcFwE9hoPaKGldf6z?=
 =?us-ascii?Q?aZ+nhUg+5YKxOxGaIwSPb78OQ1sWNrxScziDsekDtYlUkGulGBLw1P6LH+64?=
 =?us-ascii?Q?rw8QWHleKb8CBlViz5r5NFulfscujMRXSEFblLvPo9BQCCQyw5zShaSzeotp?=
 =?us-ascii?Q?tm7kglgCrdLz/xkE9RxdIHwiOmcEnoPTSSLqtt9g0JthYBMXEszB3zUW12qw?=
 =?us-ascii?Q?OUMqfO0wXqKlKq+Q28kwPrf2PhazF/TPLXYldKTSz56KgPSJvcgtgF6l2Mvh?=
 =?us-ascii?Q?xh6AharbFgOWV4C8Qeqdak922xI+JVlScjs0uz5082OnjVYiul0dUjjlbF1G?=
 =?us-ascii?Q?5eW9pDcFHhvqOcyCDlA6w92Om1/YxJs7QBZ59feI10PZu7F93uEVD3cIyIXH?=
 =?us-ascii?Q?EAKiBl2ecjSO9R3vIHCzPkfkEmz0SsZNUBnkTwM0x1wo4hf85TtglglyQQ37?=
 =?us-ascii?Q?CNx+eryxrMr8xVXKp3DkUVLXhlVU2eCORYnHmoK2dniBn9iF0Z3WEHCIVbOj?=
 =?us-ascii?Q?GhBeW98S6kGab6RGHYXW4g1mALBOj+h8OKo79MeXioeigEYfeziUyc/XkClc?=
 =?us-ascii?Q?XMvjOQxjIP5JEweSdHiQa+eOeXLNhBpGQCwgUkbSNg0kfuLcj9MlJ9v1trZA?=
 =?us-ascii?Q?ywE7HP/xYJv+xpMDHhPNuYjOfJhPfY0SI091n4fKKfGbWJd+izDDSHJ15OzH?=
 =?us-ascii?Q?MUZQoOxuZ3dEW/TNoK00qUzaPF2eGNd3mTTKewEi9eTA6Vx4xl0EYRzRBsx7?=
 =?us-ascii?Q?uv9D8V6EjqwD3eU/laBoLOyqSAaT+lM7qhyJdOsv?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 552a9bc3-ef2f-4e81-ef20-08dc62a8e30d
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6020.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Apr 2024 08:47:48.4476
 (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: 9Yvw17egSgQyrE+4vanNwOjUJ/rbA5L2rzmeXjey23pwfN0dt10tFGs/O/8mLnQMIwpI02jM0BvxVR58EzIs9A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7917
X-OriginatorOrg: intel.com

On 2024-04-22 at 16:22:38 +0800, Xuewen Yan wrote:
> kernel encounters the following error when running workload:
> 
> BUG: kernel NULL pointer dereference, address: 0000002c
> EIP: set_next_entity (fair.c:?)
> 
> which was caused by NULL pointer returned by pick_eevdf().
> 
> Further investigation has shown that, the entity_eligible() has an
> false-negative issue when the entity's vruntime is far behind the
> cfs_rq.min_vruntime that, the (vruntime - cfs_rq->min_vruntime) * load
> caused a s64 overflow, thus every entity on the rb-tree is not
> eligible, which results in a NULL candidate.
> 
> The reason why entity's vruntime is far behind the cfs_rq.min_vruntime
> is because during a on_rq task group's update_cfs_group()->reweight_eevdf(),
> there is no limit on the new entity's vlag. If the new weight is much
> smaller than the old one,
> 
> vlag = div_s64(vlag * old_weight, weight)
> 
> generates a huge vlag, and results in very small(negative) vruntime.
> 
> Thus limit the range of vlag accordingly.
>

Thanks for the fix.

Might also add comments from Tim suggested here:
https://lore.kernel.org/lkml/ec479251e6245148b89b226f734192f6d5343bbc.camel@linux.intel.com/

A fix tag might be needed.
Fixes: eab03c23c2a1 ("sched/eevdf: Fix vruntime adjustment on reweight")
 
> Reported-by: Sergei Trofimovich <slyich@gmail.com>
> Closes: https://lore.kernel.org/all/ZhuYyrh3mweP_Kd8@nz.home/
> Reported-by: Igor Raits <igor@gooddata.com>
> Closes: https://lore.kernel.org/all/CA+9S74ih+45M_2TPUY_mPPVDhNvyYfy1J1ftSix+KjiTVxg8nw@mail.gmail.com/
> Reported-by: Breno Leitao <leitao@debian.org>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/lkml/202401301012.2ed95df0-oliver.sang@intel.com/
> Reported-by: Yujie Liu <yujie.liu@intel.com>
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> ---

Cced Sergei, Igor, Breno who have encountered this NULL pointer issue before.

From my testing, with this applied I did not see the NULL pointer exception
after running trinity for 100 cycles, so

Reviewed-and-tested-by: Chen Yu <yu.c.chen@intel.com>

thanks,
Chenyu

> changes of v2:
> -add reported-by (suggested by <yu.c.chen@intel.com>)
> -remork the changelog (<yu.c.chen@intel.com>)
> -remove the judgement of fork (Peter)
> -remove the !on_rq case. (Peter)
> ---
> Previous discussion link:
> https://lore.kernel.org/lkml/20240226082349.302363-1-yu.c.chen@intel.com/
> https://lore.kernel.org/all/20240130080643.1828-1-xuewen.yan@unisoc.com/
> ---
> ---
>  kernel/sched/fair.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 03be0d1330a6..64826f406d6d 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -696,15 +696,21 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq)
>   *
>   * XXX could add max_slice to the augmented data to track this.
>   */
> -static void update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
> +static s64 entity_lag(u64 avruntime, struct sched_entity *se)
>  {
> -	s64 lag, limit;
> +	s64 vlag, limit;
> +
> +	vlag = avruntime - se->vruntime;
> +	limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se);
> +
> +	return clamp(vlag, -limit, limit);
> +}
>  
> +static void update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
> +{
>  	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 = entity_lag(avg_vruntime(cfs_rq), se);
>  }
>  
>  /*
> @@ -3761,7 +3767,7 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  	 *	   = V  - vl'
>  	 */
>  	if (avruntime != se->vruntime) {
> -		vlag = (s64)(avruntime - se->vruntime);
> +		vlag = entity_lag(avruntime, se);
>  		vlag = div_s64(vlag * old_weight, weight);
>  		se->vruntime = avruntime - vlag;
>  	}
> -- 
> 2.25.1
>