Received-SPF: pass (google.com: domain of linux-kernel+bounces-213307-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 13 Jun 2024 21:23:04 +0800
From: Chen Yu <yu.c.chen@intel.com>
To: Chunxin Zang <spring.cxz@gmail.com>
CC: Honglei Wang <jameshongleiwang@126.com>, <mingo@redhat.com>, "Peter
 Zijlstra" <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>, Mike Galbraith <efault@gmx.de>,
	K Prateek Nayak <kprateek.nayak@amd.com>, <linux-kernel@vger.kernel.org>,
	<yangchen11@lixiang.com>, Jerry Zhou <zhouchunhua@lixiang.com>, Chunxin Zang
	<zangchunxin@lixiang.com>
Subject: Re: [PATCH v2] sched/fair: Reschedule the cfs_rq when current is
 ineligible
Message-ID: <ZmryuDqw2fEhHNni@chenyu5-mobl2>
References: <20240529141806.16029-1-spring.cxz@gmail.com>
 <ZmKVjmuC4kGrUH5V@chenyu5-mobl2>
 <2E6EB0D6-D913-4205-B7DD-35EF4FA25667@gmail.com>
 <Zmrd2hwqHB8RQjqB@chenyu5-mobl2>
 <112FECA8-5D21-406F-814C-ACBE63351CBB@gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <112FECA8-5D21-406F-814C-ACBE63351CBB@gmail.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?BwNIgEnFfDi8rPGgH7BCyCTObSRHWuohsVQrewPBu0zD2tnyT7xWxXO4f4hM?=
 =?us-ascii?Q?e9CyrHf59nvBrJQTh/8hh3SV8jLln9r2jWTaVYuBD63i3V/ri2d6MiCvxoMU?=
 =?us-ascii?Q?uJufzf3QxniOsY4ME7fg3F92dzOafjl28kOX/TLaJf2qIrorrPzYUGgcGzOL?=
 =?us-ascii?Q?6kS1DDOVRe6Tg+oSG/nCU0rmXkm/OzdtOG6X50O7qRIYLOacHaG44lobWJIf?=
 =?us-ascii?Q?XC+IbEc+NBbsbBhhjnO9ON61/V0WM7uSN2fwth+zSmk90xyEm1ukMtkFQg1X?=
 =?us-ascii?Q?nSXobaC/R01kZgRVHQj8i0QuesCPbewfW0ojliH693i/lOaKJH0Y8OBhgUQV?=
 =?us-ascii?Q?j6PmTbUtTTaED1SsF+sRdbFQ5i0sZ0/ml39+tOut/WI//eIbDMZc3TRsPo57?=
 =?us-ascii?Q?gHndoaeo5hmH9zOy/Ayn/kCXxoZSOoLZfkXxrievoWEbHav3ptA1DWfnsSC/?=
 =?us-ascii?Q?iTXYVJuxXDHn9MAjKA+l14H1J8iOrmKjgVbYIS8xqdIw9bW+t10dNDSuBXO3?=
 =?us-ascii?Q?B0Q7eMNQ4GVYfhAhxIqRdF047Dd5lI/ci3Gi3wkpfc6iNjm6ISokTS3X21aF?=
 =?us-ascii?Q?GWRHl8kpd0taR3VRqnBPM2oyoP621Ns0YQRq49MiwivCqfMZxOZrNqtWmlcK?=
 =?us-ascii?Q?8waQj/VqHWnjlZKeR8dxXpj4OMCnxK7TXSh1azVS/Fq6lfnxT9QsmYVlcHyb?=
 =?us-ascii?Q?4cTlHWEIrzYsIqHTnmMyzsu5wiWFFJgMkPNCLQu8aa9SvmN5shPweLNX207b?=
 =?us-ascii?Q?n3FbtbkXsnJYc8csjxuIuPKQOBckGaGXfUJwsPB1ulomj1Awi+oAVEDKK1oQ?=
 =?us-ascii?Q?rSA7VKmMXJE9TUqXBg8MxHmCy5oiJ/bK+MYo83EwLK1Vt96q8PsJprRXvsAO?=
 =?us-ascii?Q?Sv8ELRMUlUMiH67V6NlNMb8LyyfqT4T5zyh7tPuv+gkG+Brjf8+eCCcUsCsK?=
 =?us-ascii?Q?7iz+tZFc3oXw0Dng1jcSb7J4GBBYVQIL2bc+ktms4ndOsgH5ZfRcK/cv9CI5?=
 =?us-ascii?Q?d+TAsYwcrB4bivKdGCgVo0pip580TAZ9Ia3U5yQrqvoGFamVvCQB9H4IQQQE?=
 =?us-ascii?Q?v0W4ejmf5+lEInpJ+Uz3U4Q5XWoeUg3jVSAnuMq2mgAk9NxapZRNTvx/1nXd?=
 =?us-ascii?Q?el5AA1GRn6Hko+M1Dh/AnojnoKJU+W9KaxgGBQorjGRGkPNSdL/7zsdF6pxH?=
 =?us-ascii?Q?1GVASknuf3pH2c0WeanFDyX8soJX2gMt1jeoftdoFPs7GGaEgfG9Fhm22AB9?=
 =?us-ascii?Q?PEdzN17XTvIde9rqoxPGSZNXoYwBUe6YEz4NJQ76EXMIXOU1GG6hJlbRE5QV?=
 =?us-ascii?Q?zRpyq5S86wSFqxNuJEWfXRNzPJfmgBTpBM8ZtiWVfCbsa2XMGm4rK9kT2f2d?=
 =?us-ascii?Q?wuptbLYoVH31C3obS0V/4/PUpU3jlDKXiMzDEVDJsbPfKEx7BBYw8zWzCCBJ?=
 =?us-ascii?Q?t0ZC4qwaKPUKRgcP+lkoIA/hndjPvrDyv3iQJtoBSvz/SG/MV7+FDY8DSTRm?=
 =?us-ascii?Q?r6Mi5u25hNgaARusMUOsIWB+YFWVgaqbTu0ibagzgieCCUUxEqXfchJgKiQi?=
 =?us-ascii?Q?26iFbozqUZII62IZlnWVAxgfyDRENEzTRx0qlIR1?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 94e0346c-5b76-4a34-8932-08dc8babfc75
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6020.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jun 2024 13:23:17.2312
 (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: 1WLl0T141MM+Yop5ngxs/uZqAFDFX0PwMCzQnsCZA9XQ7StH77VNJJij1K+Q324siLPF62a6mYxfa+MCcBsjDQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB7021
X-OriginatorOrg: intel.com

On 2024-06-13 at 20:02:37 +0800, Chunxin Zang wrote:
> 
> 
> > On Jun 13, 2024, at 19:54, Chen Yu <yu.c.chen@intel.com> wrote:
> > 
> > On 2024-06-12 at 18:39:11 +0800, Chunxin Zang wrote:
> >> 
> >> 
> >>> On Jun 7, 2024, at 13:07, Chen Yu <yu.c.chen@intel.com> wrote:
> >>> 
> >>> On 2024-05-29 at 22:18:06 +0800, Chunxin Zang wrote:
> >> The purpose of the modification is to increase preemption opportunities without breaking the
> >> RUN_TO_PARITY rule. However, it clearly introduces some additional preemptions, or perhaps
> >> there should be a check for the eligibility of the se. Also, to avoid overwriting the scheduling
> >> strategy in entity_tick, would a modification like the following be more appropriate?
> >> 
> > 
> > I wonder if we can only take care of the NO_RUN_TO_PARITY case? Something like this,
> > 
> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> >> index 03be0d1330a6..5e49a15bbdd3 100644
> >> --- a/kernel/sched/fair.c
> >> +++ b/kernel/sched/fair.c
> >> @@ -745,6 +745,21 @@ int entity_eligible(struct cfs_rq *cfs_rq, struct sched_entity *se)
> >>        return vruntime_eligible(cfs_rq, se->vruntime);
> >> }
> >> 
> >> +static bool check_entity_need_preempt(struct cfs_rq *cfs_rq, struct sched_entity *se)
> >> +{
> > if (sched_feat(RUN_TO_PARITY) || cfs_rq->nr_running <= 1 ||
> >     !entity_eligible(cfs_rq, se))
> > return false;
> > 
> > return true;
> > 
> > Thoughts?
> > 
> 
> This does indeed look better. In that case, do I need to make the changes this way and send
> out a version 3?

If you mean the following changes, maybe we can continue the discussion here.
This is just my 2 cents, not sure what others think of it. Anyway, I can launch some tests.

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 8a5b1ae0aa55..c0fdb25f0695 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -744,6 +744,15 @@ int entity_eligible(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	return vruntime_eligible(cfs_rq, se->vruntime);
 }
 
+static bool check_curr_preempt(struct cfs_rq *cfs_rq, struct sched_entity *curr)
+{
+	if (sched_feat(RUN_TO_PARITY) || cfs_rq->nr_running <= 1 ||
+	    !entity_eligible(cfs_rq, curr))
+		return false;
+
+	return true;
+}
+
 static u64 __update_min_vruntime(struct cfs_rq *cfs_rq, u64 vruntime)
 {
 	u64 min_vruntime = cfs_rq->min_vruntime;
@@ -5536,6 +5545,9 @@ entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)
 			hrtimer_active(&rq_of(cfs_rq)->hrtick_timer))
 		return;
 #endif
+
+	if (check_curr_preempt(cfs_rq, curr))
+		resched_curr(rq_of(cfs_rq));
 }
 
 
@@ -8415,7 +8427,7 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
 	/*
 	 * XXX pick_eevdf(cfs_rq) != se ?
 	 */
-	if (pick_eevdf(cfs_rq) == pse)
+	if (check_curr_preempt(cfs_rq, se) || pick_eevdf(cfs_rq) == pse)
 		goto preempt;
 
 	return;
-- 
2.25.1