From: Sean Christopherson <seanjc@google.com>
To: Zide Chen <zide.chen@intel.com>
Cc: linux-kselftest@vger.kernel.org, pbonzini@redhat.com
Subject: Re: [PATCH] selftests/rseq: take large C-state exit latency into consideration
Date: Fri, 5 Apr 2024 16:01:13 -0700 [thread overview]
Message-ID: <ZhCCub4ajIvpvrBk@google.com> (raw)
In-Reply-To: <20240322163351.150673-1-zide.chen@intel.com>
On Fri, Mar 22, 2024, Zide Chen wrote:
> Currently, the migration worker delays 1-10 us, assuming that one
> KVM_RUN iteration only takes a few microseconds. But if C-state exit
> latencies are large enough, for example, hundreds or even thousands
> of microseconds on server CPUs, it may happen that it's not able to
> bring the target CPU out of C-state before the migration worker starts
> to migrate it to the next CPU.
>
> If the system workload is light, most CPUs could be at a certain level
> of C-state, and the vCPU thread may waste milliseconds before it can
> actually migrate to a new CPU.
Well fudge. That's definitely not on my bingo sheet.
> Thus, the tests may be inefficient in such systems, and in some cases
> it may fail the migration/KVM_RUN ratio sanity check.
>
> Since we are not able to turn off the cpuidle sub-system in run time,
> this patch creates an idle thread on every CPU to prevent them from
> entering C-states.
First off, huge thanks for debugging this! That must have been quite the task
(no pun intended).
While spinning up threads on every CPU is a clever way to ensure they don't go
into a deep sleep state, I'm not exactly excited about the idea of putting every
reachable CPU into a busy loop. And while this doesn't add _that_ much complexity,
I'm not sure the benefit (preserving the assert for all systems) is worth it. I
also don't want to arbitrarily prevent idle task (as in, the kernel's idle task)
interactions. E.g. it's highly (highly) unlikely, but not impossible for there
to be a bug that's unique to idle tasks, or C-states, or other edge case.
Are there any metrics/stats that can be (easily) checked to grant an exception
to the sanity check? That's a very hand-wavy question, as I'm not even sure what
type of stat we'd want to look at. Actual runtime of a task, maybe?
If that's not easy, what if we add an off-by-default command line option to skip
the sanity check? I was resistant to simply deleting the assert in the past, but
that was mainly because I didn't want to delete it without understanding what was
causing problems. That would allow CI environments to opt-out as needed, while
still keeping the sanity check alive for enough systems to make it useful.
next prev parent reply other threads:[~2024-04-05 23:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-22 16:33 [PATCH] selftests/rseq: take large C-state exit latency into consideration Zide Chen
2024-04-05 23:01 ` Sean Christopherson [this message]
2024-04-12 16:47 ` Chen, Zide
2024-04-12 18:52 ` Sean Christopherson
2024-04-12 22:16 ` Chen, Zide
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=ZhCCub4ajIvpvrBk@google.com \
--to=seanjc@google.com \
--cc=linux-kselftest@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=zide.chen@intel.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.