public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Chunxin Zang <spring.cxz@gmail.com>
Cc: mingo@redhat.com, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, yu.c.chen@intel.com,
	dietmar.eggemann@arm.com, rostedt@goodmis.org,
	bsegall@google.com, mgorman@suse.de, bristot@redhat.com,
	vschneid@redhat.com, linux-kernel@vger.kernel.org, efault@gmx.de,
	kprateek.nayak@amd.com, jameshongleiwang@126.com,
	yangchen11@lixiang.com, zangchunxin@lixiang.com
Subject: Re: [PATCH v3] sched/fair: Preempt if the current process is ineligible
Date: Thu, 20 Jun 2024 14:51:55 +0200	[thread overview]
Message-ID: <20240620125155.GY31592@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20240613131437.9555-1-spring.cxz@gmail.com>

On Thu, Jun 13, 2024 at 09:14:37PM +0800, Chunxin Zang wrote:
> ---
>  kernel/sched/fair.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 03be0d1330a6..21ef610ddb14 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -745,6 +745,15 @@ 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;
> +}
> +
>  static u64 __update_min_vruntime(struct cfs_rq *cfs_rq, u64 vruntime)
>  {
>  	u64 min_vruntime = cfs_rq->min_vruntime;
> @@ -974,11 +983,13 @@ static void clear_buddies(struct cfs_rq *cfs_rq, struct sched_entity *se);
>  /*
>   * XXX: strictly: vd_i += N*r_i/w_i such that: vd_i > ve_i
>   * this is probably good enough.
> + *
> + * return true if se need preempt
>   */
> -static void update_deadline(struct cfs_rq *cfs_rq, struct sched_entity *se)
> +static bool update_deadline(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  {
>  	if ((s64)(se->vruntime - se->deadline) < 0)
> -		return;
> +		return false;
>  
>  	/*
>  	 * For EEVDF the virtual time slope is determined by w_i (iow.
> @@ -995,10 +1006,7 @@ static void update_deadline(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  	/*
>  	 * The task has consumed its request, reschedule.
>  	 */
> -	if (cfs_rq->nr_running > 1) {
> -		resched_curr(rq_of(cfs_rq));
> -		clear_buddies(cfs_rq, se);
> -	}
> +	return true;
>  }
>  
>  #include "pelt.h"
> @@ -1157,6 +1165,7 @@ static void update_curr(struct cfs_rq *cfs_rq)
>  {
>  	struct sched_entity *curr = cfs_rq->curr;
>  	s64 delta_exec;
> +	bool need_preempt;
>  
>  	if (unlikely(!curr))
>  		return;
> @@ -1166,12 +1175,17 @@ static void update_curr(struct cfs_rq *cfs_rq)
>  		return;
>  
>  	curr->vruntime += calc_delta_fair(delta_exec, curr);
> -	update_deadline(cfs_rq, curr);
> +	need_preempt = update_deadline(cfs_rq, curr);
>  	update_min_vruntime(cfs_rq);
>  
>  	if (entity_is_task(curr))
>  		update_curr_task(task_of(curr), delta_exec);
>  
> +	if (need_preempt || check_entity_need_preempt(cfs_rq, curr)) {
> +		resched_curr(rq_of(cfs_rq));
> +		clear_buddies(cfs_rq, curr);
> +	}
> +
>  	account_cfs_rq_runtime(cfs_rq, delta_exec);
>  }

Yeah sorry no. This will mess up the steady state schedule. At best we
can do something like the below which will make PREEMPT_SHORT a little
more effective I suppose.


--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -985,10 +985,10 @@ static void clear_buddies(struct cfs_rq
  * XXX: strictly: vd_i += N*r_i/w_i such that: vd_i > ve_i
  * this is probably good enough.
  */
-static void update_deadline(struct cfs_rq *cfs_rq, struct sched_entity *se)
+static bool update_deadline(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
 	if ((s64)(se->vruntime - se->deadline) < 0)
-		return;
+		return false;
 
 	/*
 	 * For EEVDF the virtual time slope is determined by w_i (iow.
@@ -1005,10 +1005,7 @@ static void update_deadline(struct cfs_r
 	/*
 	 * The task has consumed its request, reschedule.
 	 */
-	if (cfs_rq->nr_running > 1) {
-		resched_curr(rq_of(cfs_rq));
-		clear_buddies(cfs_rq, se);
-	}
+	return true;
 }
 
 #include "pelt.h"
@@ -1168,6 +1165,8 @@ static void update_curr(struct cfs_rq *c
 {
 	struct sched_entity *curr = cfs_rq->curr;
 	s64 delta_exec;
+	struct rq *rq;
+	bool resched;
 
 	if (unlikely(!curr))
 		return;
@@ -1177,13 +1176,23 @@ static void update_curr(struct cfs_rq *c
 		return;
 
 	curr->vruntime += calc_delta_fair(delta_exec, curr);
-	update_deadline(cfs_rq, curr);
+	resched = update_deadline(cfs_rq, curr);
 	update_min_vruntime(cfs_rq);
 
 	if (entity_is_task(curr))
 		update_curr_task(task_of(curr), delta_exec);
 
 	account_cfs_rq_runtime(cfs_rq, delta_exec);
+
+	rq = rq_of(cfs_rq);
+	if (rq->nr_running == 1)
+		return;
+
+	if (resched ||
+	    (curr->vlag != curr->deadline && !entity_eligible(cfs_rq, curr))) {
+		resched_curr(rq);
+		clear_buddies(cfs_rq, curr);
+	}
 }
 
 static void update_curr_fair(struct rq *rq)

  reply	other threads:[~2024-06-20 12:52 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-13 13:14 [PATCH v3] sched/fair: Preempt if the current process is ineligible Chunxin Zang
2024-06-20 12:51 ` Peter Zijlstra [this message]
2024-06-21 13:53   ` Chunxin Zang
     [not found]     ` <36B22124-E952-4508-A4A3-5AE2C944FBDF@gmail.com>
     [not found]       ` <9e56b874-724e-4c2e-8e7d-db6317cb414c@gmail.com>
2024-07-15 13:05         ` John Stills
2024-07-16  9:52           ` Chunxin Zang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240620125155.GY31592@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=efault@gmx.de \
    --cc=jameshongleiwang@126.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=spring.cxz@gmail.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=yangchen11@lixiang.com \
    --cc=yu.c.chen@intel.com \
    --cc=zangchunxin@lixiang.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox