Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu_topology_test: Add test case to test cpu topology #2337

Merged
merged 1 commit into from Sep 2, 2020

Conversation

vivianQizhu
Copy link
Contributor

id: 1856239
Signed-off-by: Qianqian Zhu qizhu@redhat.com

@vivianQizhu vivianQizhu force-pushed the cpu_topology branch 4 times, most recently from 252e902 to 6d9bf2e Compare July 30, 2020 07:02
@vivianQizhu vivianQizhu force-pushed the cpu_topology branch 2 times, most recently from 911e248 to 685688b Compare August 3, 2020 06:42
@vivianQizhu
Copy link
Contributor Author

@nanliu-r Updated, please help review. @Kuhn-Chen @PaulYuuu Please help review as well if you are available, thanks.

qemu/tests/cpu_topology_test.py Outdated Show resolved Hide resolved
qemu/tests/cpu_topology_test.py Outdated Show resolved Hide resolved
@vivianQizhu vivianQizhu force-pushed the cpu_topology branch 2 times, most recently from 63760f2 to 5ea3681 Compare August 3, 2020 10:41
@nanliu-r
Copy link
Contributor

nanliu-r commented Aug 4, 2020

Hi, guest refuses to start for this error when the host has 4 CPUs, please help to check this and do we need to boot guest with 240 smp on every host?

/usr/libexec/qemu-kvm -smp 240,maxcpus=240,cores=5,threads=1,dies=1,sockets=48 -M q35
VNC server running on ::1:5900
qemu-kvm: current -smp configuration requires Extended Interrupt Mode enabled. You can add an IOMMU using: -device intel-iommu,intremap=on,eim=on

@vivianQizhu
Copy link
Contributor Author

Thanks @nanliu-r Updated to vcpu_sockets = min(240 // (vcpu_cores * vcpu_threads), 10), what do you think?

@nanliu-r
Copy link
Contributor

nanliu-r commented Aug 4, 2020

Thanks @nanliu-r Updated to vcpu_sockets = min(240 // (vcpu_cores * vcpu_threads), 10), what do you think?

Hi, seems we can't always get the right list, sometimes guest refuses to start for this:
-smp 70,maxcpus=70,cores=7,threads=1,dies=1,sockets=2
[qemu output] qemu-kvm: cpu topology: sockets (2) * dies (1) * cores (7) * threads (1) < smp_cpus (70)

please help to check this, thanks.

@vivianQizhu
Copy link
Contributor Author

Thanks @nanliu-r Updated to vcpu_sockets = min(240 // (vcpu_cores * vcpu_threads), 10), what do you think?

Hi, seems we can't always get the right list, sometimes guest refuses to start for this:
-smp 70,maxcpus=70,cores=7,threads=1,dies=1,sockets=2
[qemu output] qemu-kvm: cpu topology: sockets (2) * dies (1) * cores (7) * threads (1) < smp_cpus (70)

please help to check this, thanks.

Hi @nanliu-r Is this a Windows guest?

@Kuhn-Chen
Copy link
Contributor

Thanks @nanliu-r Updated to vcpu_sockets = min(240 // (vcpu_cores * vcpu_threads), 10), what do you think?

Hi, seems we can't always get the right list, sometimes guest refuses to start for this:
-smp 70,maxcpus=70,cores=7,threads=1,dies=1,sockets=2
[qemu output] qemu-kvm: cpu topology: sockets (2) * dies (1) * cores (7) * threads (1) < smp_cpus (70)

please help to check this, thanks.

According to this statement, the smp should not be greater than the (vcpu_cores * vcpu_threads * vcpu_sockets).
“ params['smp'] = params['vcpu_maxcpus'] = (vcpu_cores *
vcpu_threads * vcpu_sockets)“

@nanliu-r
Copy link
Contributor

nanliu-r commented Aug 4, 2020

Thanks @nanliu-r Updated to vcpu_sockets = min(240 // (vcpu_cores * vcpu_threads), 10), what do you think?

Hi, seems we can't always get the right list, sometimes guest refuses to start for this:
-smp 70,maxcpus=70,cores=7,threads=1,dies=1,sockets=2
[qemu output] qemu-kvm: cpu topology: sockets (2) * dies (1) * cores (7) * threads (1) < smp_cpus (70)
please help to check this, thanks.

Hi @nanliu-r Is this a Windows guest?

Yes

Thanks @nanliu-r Updated to vcpu_sockets = min(240 // (vcpu_cores * vcpu_threads), 10), what do you think?

Hi, seems we can't always get the right list, sometimes guest refuses to start for this:
-smp 70,maxcpus=70,cores=7,threads=1,dies=1,sockets=2
[qemu output] qemu-kvm: cpu topology: sockets (2) * dies (1) * cores (7) * threads (1) < smp_cpus (70)
please help to check this, thanks.

Hi @nanliu-r Is this a Windows guest?

Yes, this issue can reproduce with windows guest, and for rhel guest, I can met the issue output: 'qemu-kvm: current -smp configuration requires Extended Interrupt Mode enabled. You can add an IOMMU using: -device intel-iommu,intremap=on,eim=on' sometimes when boot guest with -smp 200,maxcpus=200,cores=10,threads=2,dies=1,sockets=10.

seems we suggest to overcommit host CPU number five times in one VM .@PaulYuuu does your case need a special machine as your comments "On ppc,the most extreme case is smp = 10 * 8 * 10, which is larger than vCPU limitation(240/384), please limit vcpu_cores and vcpu_sockets."

@vivianQizhu
Copy link
Contributor Author

seems we suggest to overcommit host CPU number five times in one VM .@PaulYuuu does your case need a special machine as your comments "On ppc,the most extreme case is smp = 10 * 8 * 10, which is larger than vCPU limitation(240/384), please limit vcpu_cores and vcpu_sockets."

I think he means the extreme case from my previous code.

@vivianQizhu vivianQizhu force-pushed the cpu_topology branch 4 times, most recently from ae06322 to b2fed6d Compare August 4, 2020 08:31
Comment on lines 44 to 46
vcpu_sockets = min(max(host_cpu * 5 // (vcpu_cores * vcpu_threads), 1),
random.randint(1, 10))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some hosts have a lot of CPU, so there is a risk:

min(max(192 * 5 // (8 * 10), 1), random.randint(1, 10))
10

And then, smp = vcpu_maxcpus = 8 * 10 * 10
I think using fixed sockets (2) is okay for this test case.

IIUC, AMD CPUs do not support multiple threads except EYPC. @nanliu-r please help to confirm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some hosts have a lot of CPU, so there is a risk:

min(max(192 * 5 // (8 * 10), 1), random.randint(1, 10))
10

And then, smp = vcpu_maxcpus = 8 * 10 * 10
I think using fixed sockets (2) is okay for this test case.

@nanliu-r What do you think about this, to have a fixed sockets(2)?

@nanliu-r
Copy link
Contributor

nanliu-r commented Aug 10, 2020

Hi, I still can meet the issue "[qemu output] qemu-kvm: cpu topology: sockets (8) * cores (2) * threads (1) < smp_cpus (32)" on EPYC (Milan) machine, both windows and rhel guest, can you help to check again?

Architecture:        x86_64
CPU op-mode(s):      32-bit, 64-bit
Byte Order:          Little Endian
CPU(s):              256
On-line CPU(s) list: 0-255
Thread(s) per core:  2
Core(s) per socket:  64
Socket(s):           2
NUMA node(s):        2
Vendor ID:           AuthenticAMD
CPU family:          25
Model:               0
Model name:          AMD Eng Sample: 100-000000114-07_22/15_N
Stepping:            0

@PaulYuuu
Copy link
Contributor

Hi, I still can meet the issue "[qemu output] qemu-kvm: cpu topology: sockets (8) * cores (2) * threads (1) < smp_cpus (32)" on EPYC (Milan) machine, both windows and rhel guest, can you help to check again?

Architecture:        x86_64
CPU op-mode(s):      32-bit, 64-bit
Byte Order:          Little Endian
CPU(s):              256
On-line CPU(s) list: 0-255
Thread(s) per core:  2
Core(s) per socket:  64
Socket(s):           2
NUMA node(s):        2
Vendor ID:           AuthenticAMD
CPU family:          25
Model:               0
Model name:          AMD Eng Sample: 100-000000114-07_22/15_N
Stepping:            0

This is the known issue of all AMD platforms. https://github.com/avocado-framework/avocado-vt/blob/666b5e5bb353272835c8d6bf618e11e8890fff3e/virttest/qemu_vm.py#L1532-L1540

@nanliu-r
Copy link
Contributor

nanliu-r commented Aug 10, 2020

Hi, I still can meet the issue "[qemu output] qemu-kvm: cpu topology: sockets (8) * cores (2) * threads (1) < smp_cpus (32)" on EPYC (Milan) machine, both windows and rhel guest, can you help to check again?

Architecture:        x86_64
CPU op-mode(s):      32-bit, 64-bit
Byte Order:          Little Endian
CPU(s):              256
On-line CPU(s) list: 0-255
Thread(s) per core:  2
Core(s) per socket:  64
Socket(s):           2
NUMA node(s):        2
Vendor ID:           AuthenticAMD
CPU family:          25
Model:               0
Model name:          AMD Eng Sample: 100-000000114-07_22/15_N
Stepping:            0

This is the known issue of all AMD platforms. https://github.com/avocado-framework/avocado-vt/blob/666b5e5bb353272835c8d6bf618e11e8890fff3e/virttest/qemu_vm.py#L1532-L1540

@vivianQizhu Hi, since EPYC support multi-threads now can we do some changes in vt?

Besides, I still can reproduce this error on intel machine with windows guest with probability (6/10)

Architecture:        x86_64
CPU op-mode(s):      32-bit, 64-bit
Byte Order:          Little Endian
CPU(s):              4
On-line CPU(s) list: 0-3
Thread(s) per core:  1
Core(s) per socket:  4
Socket(s):           1
NUMA node(s):        1
Vendor ID:           GenuineIntel
CPU family:          6
Model:               94
Model name:          Intel(R) Xeon(R) CPU E3-1225 v5 @ 3.30GHz
Stepping:            3

@vivianQizhu vivianQizhu force-pushed the cpu_topology branch 4 times, most recently from b6a4148 to b48a505 Compare August 19, 2020 08:51
@vivianQizhu
Copy link
Contributor Author

@nanliu-r Any more comments?

@vivianQizhu vivianQizhu force-pushed the cpu_topology branch 2 times, most recently from e93b2e6 to f236198 Compare September 1, 2020 06:24
if params['machine_type'] == 'pseries':
vcpu_threads_list = [1, 2, 4, 8]
params['vcpu_cores'] = vcpu_cores = random.randint(1, 10)
host_cpu = cpu.online_count()
Copy link
Contributor

@nanliu-r nanliu-r Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get the vcpu_cores according to the host_cpu to avoid CPU overcommit?
I mean if the host_cpu is less than 10 we can decrease the range?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanliu-r Sure, what do you expect? Same as host cores?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanliu-r Sure, what do you expect? Same as host cores?

Sounds good for me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about half of host_cpu considering of multiple threads?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanliu-r What if host cpu core is 2? Will that be too small? Let's try with =host cpu and see if it works well first, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanliu-r Update to vcpu_cores = random.randint(1, max(6, host_cpu//2)). Please help review, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be vcpu_cores = random.randint(1, min(6, host_cpu//2)) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanliu-r No, you said, when host_cpu<12(hostcpu//2 < 6), it should be 6, when host_cpu >=12(host_cpu//2 > 6), it should be host_cpu//2, so it should be max(6, host_cpu//2).

Copy link
Contributor

@nanliu-r nanliu-r Sep 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vivianQizhu vcpu_cores = random.randint(1, min(6, host_cpu//2)) This is what I want to say at first.
If we use max(6, host_cpu//2) we maybe get a large maxcpus and we will hit the issue intel-iommu.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanliu-r Okay, updated.

@vivianQizhu vivianQizhu force-pushed the cpu_topology branch 2 times, most recently from c5308b2 to 806d383 Compare September 2, 2020 05:35
Signed-off-by: Qianqian Zhu <qizhu@redhat.com>
@nanliu-r
Copy link
Contributor

nanliu-r commented Sep 2, 2020

Test pass on AMD and Intel. Thanks.
(5/6) repeat3.Host_RHEL.m8.u3.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.3.0.x86_64.io-github-autotest-qemu.cpu_topology_test.q35: PASS (73.69 s)
(6/6) repeat3.Host_RHEL.m8.u3.product_rhel.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.x86_64.io-github-autotest-qemu.cpu_topology_test.q35: PASS (201.28 s)
RESULTS : PASS 6 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0

Ack.

@vivianQizhu
Copy link
Contributor Author

Thanks @nanliu-r @PaulYuuu @Kuhn-Chen

@vivianQizhu vivianQizhu merged commit b269495 into autotest:master Sep 2, 2020
zhencliu added a commit to zhencliu/tp-qemu that referenced this pull request Sep 3, 2020
@vivianQizhu vivianQizhu deleted the cpu_topology branch September 8, 2020 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants