* [PATCH V4] KVM: selftests: Add a new option to rseq_test
@ 2024-04-29 23:34 Zide Chen
2024-05-02 17:40 ` Sean Christopherson
0 siblings, 1 reply; 3+ messages in thread
From: Zide Chen @ 2024-04-29 23:34 UTC (permalink / raw
To: linux-kselftest; +Cc: seanjc, pbonzini, Zide Chen, donsheng
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(-)
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);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V4] KVM: selftests: Add a new option to rseq_test
2024-04-29 23:34 [PATCH V4] KVM: selftests: Add a new option to rseq_test Zide Chen
@ 2024-05-02 17:40 ` Sean Christopherson
2024-05-02 21:39 ` Chen, Zide
0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2024-05-02 17:40 UTC (permalink / raw
To: Zide Chen; +Cc: linux-kselftest, pbonzini, donsheng
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!
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>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V4] KVM: selftests: Add a new option to rseq_test
2024-05-02 17:40 ` Sean Christopherson
@ 2024-05-02 21:39 ` Chen, Zide
0 siblings, 0 replies; 3+ messages in thread
From: Chen, Zide @ 2024-05-02 21:39 UTC (permalink / raw
To: Sean Christopherson; +Cc: linux-kselftest, pbonzini, donsheng
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>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-02 21:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-29 23:34 [PATCH V4] KVM: selftests: Add a new option to rseq_test Zide Chen
2024-05-02 17:40 ` Sean Christopherson
2024-05-02 21:39 ` Chen, Zide
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.