| Message ID | 20240429233435.19003-1-zide.chen@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V4] KVM: selftests: Add a new option to rseq_test | expand |
On Mon, Apr 29, 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 the CPU low > power wakeup latency is large enough, for example, hundreds or even > thousands of microseconds deep C-state exit latencies on x86 server > CPUs, it may happen that it's not able to wakeup the target CPU before > the migration worker starts to migrate the vCPU thread to the next CPU. > > If the system workload is light, most CPUs could be at a certain low > power state, which may result in less successful migrations and fail the > migration/KVM_RUN ratio sanity check. But this is not supposed to be > deemed a test failure. > > This patch adds a command line option to skip the sanity check in > this case. > > Signed-off-by: Zide Chen <zide.chen@intel.com> > Co-developed-by: donsheng <dongsheng.x.zhang@intel.com> This needs Dongsheng's SoB, and your SoB should come last. And the attributed name for any tag should use the person's full name. Given that I have emails from Dongsheng that show up as "Dongsheng Zhang", I _assume_ "donsheng" is incomplete, but that's a big assumption on my part. Dongsheng, can you provide your explicit SoB, with how you want your name to show up? Thanks!
On 5/2/2024 10:40 AM, Sean Christopherson wrote: > On Mon, Apr 29, 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 the CPU low >> power wakeup latency is large enough, for example, hundreds or even >> thousands of microseconds deep C-state exit latencies on x86 server >> CPUs, it may happen that it's not able to wakeup the target CPU before >> the migration worker starts to migrate the vCPU thread to the next CPU. >> >> If the system workload is light, most CPUs could be at a certain low >> power state, which may result in less successful migrations and fail the >> migration/KVM_RUN ratio sanity check. But this is not supposed to be >> deemed a test failure. >> >> This patch adds a command line option to skip the sanity check in >> this case. >> >> Signed-off-by: Zide Chen <zide.chen@intel.com> >> Co-developed-by: donsheng <dongsheng.x.zhang@intel.com> > > This needs Dongsheng's SoB, and your SoB should come last. And the attributed > name for any tag should use the person's full name. Given that I have emails from > Dongsheng that show up as "Dongsheng Zhang", I _assume_ "donsheng" is incomplete, > but that's a big assumption on my part. > > Dongsheng, can you provide your explicit SoB, with how you want your name to show > up? Thanks! My bad, will get it fixed. Confirmed with Dongsheng that going forward, he will use his full name in all the tags. > From Documentation/process/submitting-patches.rst: > > Co-developed-by: states that the patch was co-created by multiple developers; > it is used to give attribution to co-authors (in addition to the author > attributed by the From: tag) when several people work on a single patch. Since > Co-developed-by: denotes authorship, every Co-developed-by: must be immediately > followed by a Signed-off-by: of the associated co-author. Standard sign-off > procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the > chronological history of the patch insofar as possible, regardless of whether > the author is attributed via From: or Co-developed-by:. Notably, the last > Signed-off-by: must always be that of the developer submitting the patch. > > Note, the From: tag is optional when the From: author is also the person (and > email) listed in the From: line of the email header. > > Example of a patch submitted by the From: author:: > > <changelog> > > Co-developed-by: First Co-Author <first@coauthor.example.org> > Signed-off-by: First Co-Author <first@coauthor.example.org> > Co-developed-by: Second Co-Author <second@coauthor.example.org> > Signed-off-by: Second Co-Author <second@coauthor.example.org> > Signed-off-by: From Author <from@author.example.org> > > Example of a patch submitted by a Co-developed-by: author:: > > From: From Author <from@author.example.org> > > <changelog> > > Co-developed-by: Random Co-Author <random@coauthor.example.org> > Signed-off-by: Random Co-Author <random@coauthor.example.org> > Signed-off-by: From Author <from@author.example.org> > Co-developed-by: Submitting Co-Author <sub@coauthor.example.org> > Signed-off-by: Submitting Co-Author <sub@coauthor.example.org> >
diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c index 28f97fb52044..ad418a5c59dd 100644 --- a/tools/testing/selftests/kvm/rseq_test.c +++ b/tools/testing/selftests/kvm/rseq_test.c @@ -186,12 +186,35 @@ static void calc_min_max_cpu(void) "Only one usable CPU, task migration not possible"); } +static void help(const char *name) +{ + puts(""); + printf("usage: %s [-h] [-u]\n", name); + printf(" -u: Don't sanity check the number of successful KVM_RUNs\n"); + puts(""); + exit(0); +} + int main(int argc, char *argv[]) { int r, i, snapshot; struct kvm_vm *vm; struct kvm_vcpu *vcpu; u32 cpu, rseq_cpu; + bool skip_sanity_check = false; + int opt; + + while ((opt = getopt(argc, argv, "hu")) != -1) { + switch (opt) { + case 'u': + skip_sanity_check = true; + break; + case 'h': + default: + help(argv[0]); + break; + } + } r = sched_getaffinity(0, sizeof(possible_mask), &possible_mask); TEST_ASSERT(!r, "sched_getaffinity failed, errno = %d (%s)", errno, @@ -254,9 +277,17 @@ int main(int argc, char *argv[]) * getcpu() to stabilize. A 2:1 migration:KVM_RUN ratio is a fairly * conservative ratio on x86-64, which can do _more_ KVM_RUNs than * migrations given the 1us+ delay in the migration task. + * + * Another reason why it may have small migration:KVM_RUN ratio is that, + * on systems with large low power mode wakeup latency, it may happen + * quite often that the scheduler is not able to wake up the target CPU + * before the vCPU thread is scheduled to another CPU. */ - TEST_ASSERT(i > (NR_TASK_MIGRATIONS / 2), - "Only performed %d KVM_RUNs, task stalled too much?", i); + TEST_ASSERT(skip_sanity_check || i > (NR_TASK_MIGRATIONS / 2), + "Only performed %d KVM_RUNs, task stalled too much? \n" + " Try disabling deep sleep states to reduce CPU wakeup latency,\n" + " e.g. via cpuidle.off=1 or setting /dev/cpu_dma_latency to '0',\n" + " or run with -u to disable this sanity check.", i); pthread_join(migration_thread, NULL);
Currently, the migration worker delays 1-10 us, assuming that one KVM_RUN iteration only takes a few microseconds. But if the CPU low power wakeup latency is large enough, for example, hundreds or even thousands of microseconds deep C-state exit latencies on x86 server CPUs, it may happen that it's not able to wakeup the target CPU before the migration worker starts to migrate the vCPU thread to the next CPU. If the system workload is light, most CPUs could be at a certain low power state, which may result in less successful migrations and fail the migration/KVM_RUN ratio sanity check. But this is not supposed to be deemed a test failure. This patch adds a command line option to skip the sanity check in this case. Signed-off-by: Zide Chen <zide.chen@intel.com> Co-developed-by: donsheng <dongsheng.x.zhang@intel.com> --- V2: - removed the busy loop implementation - add the new "-s" option V3: - drop the usleep randomization code - removed the term C-state for less confusion for non-x86 archetectures - changed patch subject v4: - replaced Signed-off-by with Co-developed-by - changed command line option from "-s" to "-u" - Adopted the much clearer assertion error messages provided by Sean. --- tools/testing/selftests/kvm/rseq_test.c | 35 +++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-)