* [PATCH AUTOSEL 5.4 1/2] sched: Don't try to catch up excess steal time.
@ 2025-01-26 14:51 Sasha Levin
2025-01-26 14:51 ` [PATCH AUTOSEL 5.4 2/2] x86/amd_nb: Restrict init function to AMD-based systems Sasha Levin
0 siblings, 1 reply; 2+ messages in thread
From: Sasha Levin @ 2025-01-26 14:51 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Suleiman Souhlal, Peter Zijlstra, Sasha Levin, mingo, juri.lelli,
vincent.guittot
From: Suleiman Souhlal <suleiman@google.com>
[ Upstream commit 108ad0999085df2366dd9ef437573955cb3f5586 ]
When steal time exceeds the measured delta when updating clock_task, we
currently try to catch up the excess in future updates.
However, this results in inaccurate run times for the future things using
clock_task, in some situations, as they end up getting additional steal
time that did not actually happen.
This is because there is a window between reading the elapsed time in
update_rq_clock() and sampling the steal time in update_rq_clock_task().
If the VCPU gets preempted between those two points, any additional
steal time is accounted to the outgoing task even though the calculated
delta did not actually contain any of that "stolen" time.
When this race happens, we can end up with steal time that exceeds the
calculated delta, and the previous code would try to catch up that excess
steal time in future clock updates, which is given to the next,
incoming task, even though it did not actually have any time stolen.
This behavior is particularly bad when steal time can be very long,
which we've seen when trying to extend steal time to contain the duration
that the host was suspended [0]. When this happens, clock_task stays
frozen, during which the running task stays running for the whole
duration, since its run time doesn't increase.
However the race can happen even under normal operation.
Ideally we would read the elapsed cpu time and the steal time atomically,
to prevent this race from happening in the first place, but doing so
is non-trivial.
Since the time between those two points isn't otherwise accounted anywhere,
neither to the outgoing task nor the incoming task (because the "end of
outgoing task" and "start of incoming task" timestamps are the same),
I would argue that the right thing to do is to simply drop any excess steal
time, in order to prevent these issues.
[0] https://lore.kernel.org/kvm/20240820043543.837914-1-suleiman@google.com/
Signed-off-by: Suleiman Souhlal <suleiman@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20241118043745.1857272-1-suleiman@google.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/sched/core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 51ac62637e4ed..39ce8a3d8c573 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -176,13 +176,15 @@ static void update_rq_clock_task(struct rq *rq, s64 delta)
#endif
#ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
if (static_key_false((¶virt_steal_rq_enabled))) {
- steal = paravirt_steal_clock(cpu_of(rq));
+ u64 prev_steal;
+
+ steal = prev_steal = paravirt_steal_clock(cpu_of(rq));
steal -= rq->prev_steal_time_rq;
if (unlikely(steal > delta))
steal = delta;
- rq->prev_steal_time_rq += steal;
+ rq->prev_steal_time_rq = prev_steal;
delta -= steal;
}
#endif
--
2.39.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH AUTOSEL 5.4 2/2] x86/amd_nb: Restrict init function to AMD-based systems
2025-01-26 14:51 [PATCH AUTOSEL 5.4 1/2] sched: Don't try to catch up excess steal time Sasha Levin
@ 2025-01-26 14:51 ` Sasha Levin
0 siblings, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2025-01-26 14:51 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Yazen Ghannam, Borislav Petkov, Sasha Levin, tglx, mingo,
dave.hansen, x86, Shyam-sundar.S-k, richard.gong,
mario.limonciello
From: Yazen Ghannam <yazen.ghannam@amd.com>
[ Upstream commit bee9e840609cc67d0a7d82f22a2130fb7a0a766d ]
The code implicitly operates on AMD-based systems by matching on PCI
IDs. However, the use of these IDs is going away.
Add an explicit CPU vendor check instead of relying on PCI IDs.
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/20241206161210.163701-3-yazen.ghannam@amd.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/kernel/amd_nb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index e3b5de7b95988..ead78f981345c 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -538,6 +538,10 @@ static __init void fix_erratum_688(void)
static __init int init_amd_nbs(void)
{
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
+ boot_cpu_data.x86_vendor != X86_VENDOR_HYGON)
+ return 0;
+
amd_cache_northbridges();
amd_cache_gart();
--
2.39.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-01-26 14:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-26 14:51 [PATCH AUTOSEL 5.4 1/2] sched: Don't try to catch up excess steal time Sasha Levin
2025-01-26 14:51 ` [PATCH AUTOSEL 5.4 2/2] x86/amd_nb: Restrict init function to AMD-based systems Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox