All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <andrea.righi@linux.dev>
To: Tejun Heo <tj@kernel.org>, David Vernet <void@manifault.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] sched_ext: Always call put_prev_task() with scx enabled
Date: Sun, 13 Oct 2024 19:39:28 +0200	[thread overview]
Message-ID: <20241013173928.20738-1-andrea.righi@linux.dev> (raw)

With the consolidation of put_prev_task/set_next_task(), we are now
skipping the sched_ext ops.stopping/running() transitions when the
previous and next tasks are the same, see commit 436f3eed5c69 ("sched:
Combine the last put_prev_task() and the first set_next_task()").

While this optimization makes sense in general, it can negatively impact
performance in some user-space schedulers, that expect to handle such
transitions when tasks exhaust their timeslice (see SCX_OPS_ENQ_LAST).

For example, scx_rustland suffers a significant performance regression
(e.g., gaming benchmarks drop from ~60fps to ~10fps).

To fix this, ensure that put_prev_task()/set_next_task() are never
skipped when the scx scheduling class is enabled, allowing the scx class
to handle such transitions.

This change restores the previous behavior, fixing the performance
regression in scx_rustland.

Link: https://github.com/sched-ext/scx/issues/788
Fixes: 7c65ae81ea86 ("sched_ext: Don't call put_prev_task_scx() before picking the next task")
Signed-off-by: Andrea Righi <andrea.righi@linux.dev>
---
 kernel/sched/sched.h | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 6085ef50febf..44d736e49d06 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2470,21 +2470,6 @@ __put_prev_set_next_dl_server(struct rq *rq,
 	rq->dl_server = NULL;
 }
 
-static inline void put_prev_set_next_task(struct rq *rq,
-					  struct task_struct *prev,
-					  struct task_struct *next)
-{
-	WARN_ON_ONCE(rq->curr != prev);
-
-	__put_prev_set_next_dl_server(rq, prev, next);
-
-	if (next == prev)
-		return;
-
-	prev->sched_class->put_prev_task(rq, prev, next);
-	next->sched_class->set_next_task(rq, next, true);
-}
-
 /*
  * Helper to define a sched_class instance; each one is placed in a separate
  * section which is ordered by the linker script:
@@ -2523,6 +2508,21 @@ DECLARE_STATIC_KEY_FALSE(__scx_switched_all);	/* all fair class tasks on SCX */
 #define scx_switched_all()	false
 #endif /* !CONFIG_SCHED_CLASS_EXT */
 
+static inline void put_prev_set_next_task(struct rq *rq,
+					  struct task_struct *prev,
+					  struct task_struct *next)
+{
+	WARN_ON_ONCE(rq->curr != prev);
+
+	__put_prev_set_next_dl_server(rq, prev, next);
+
+	if (next == prev && !scx_enabled())
+		return;
+
+	prev->sched_class->put_prev_task(rq, prev, next);
+	next->sched_class->set_next_task(rq, next, true);
+}
+
 /*
  * Iterate only active classes. SCX can take over all fair tasks or be
  * completely disabled. If the former, skip fair. If the latter, skip SCX.
-- 
2.47.0


             reply	other threads:[~2024-10-13 17:39 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-13 17:39 Andrea Righi [this message]
2024-10-14  8:36 ` [PATCH] sched_ext: Always call put_prev_task() with scx enabled Peter Zijlstra
2024-10-14 15:00   ` Andrea Righi
2024-10-14 15:43   ` Andrea Righi

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=20241013173928.20738-1-andrea.righi@linux.dev \
    --to=andrea.righi@linux.dev \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=void@manifault.com \
    --cc=vschneid@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.