Skip to contentSkip to conversation

runtime: set MAX_SMP to number of online cpus

PATCH Applied Andrew Jones 3 participants 7 messages Nov 20, 2019 – Nov 27, 2019 KVM

lore
Share & tools
lore

Andrew Jones

author
#0 lore raw

PATCH 1/1 runtime: set MAX_SMP to number of online cpus

We can only use online cpus, so make sure we check specifically for those.

Signed-off-by Andrew Jones <[email protected]>
---
 scripts/runtime.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/runtime.bash +1 -1
index 200d5b67290c..fbad0bd05fc5 100644
@@ -1,5 +1,5 @@
 : "${RUNTIME_arch_run?}"
-: ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
+: ${MAX_SMP:=$(getconf _NPROCESSORS_ONLN)}
 : ${TIMEOUT:=90s}
 
 PASS() { echo -ne "\e[32mPASS\e[0m"; }
Signature · 1 line
2.21.0
#1 lore raw

Hi,

Quoted 13 lines · Andrew Jones

We can only use online cpus, so make sure we check specifically for those.

Signed-off-by Andrew Jones <[email protected]>
---
 scripts/runtime.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/runtime.bash +0 -0
index 200d5b67290c..fbad0bd05fc5 100644

scripts/runtime.bash line 2

Quoted 3 lines
scripts/runtime.bash +1 -1
@@ -1,5 +1,5 @@
 : "${RUNTIME_arch_run?}"
-: ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
+: ${MAX_SMP:=$(getconf _NPROCESSORS_ONLN)}

I tested it on my machine by offlining a CPU and calling getconf _NPROCESSORS_CONF (returned 32) and getconf _NPROCESSORS_ONLN (returned 31). man 3 sysconf also agrees with your patch.

I am wondering though, if _NPROCESSORS_CONF is 8 and _NPROCESSORS_ONLN is 1 (meaning that 7 CPUs were offlined), that means that qemu will create 8 VCPUs which will share the same physical CPU. Is that undesirable?

Thanks,
Alex

Quoted 3 lines
scripts/runtime.bash +0 -0
 : ${TIMEOUT:=90s}
 
 PASS() { echo -ne "\e[32mPASS\e[0m"; }
author
#2 lore raw
Quoted 24 lines · Alexandru Elisei

Hi,

Quoted 13 lines · Andrew Jones

We can only use online cpus, so make sure we check specifically for those.

Signed-off-by Andrew Jones <[email protected]>
---
 scripts/runtime.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/runtime.bash +0 -0
index 200d5b67290c..fbad0bd05fc5 100644

scripts/runtime.bash line 2

Quoted 3 lines
scripts/runtime.bash +1 -1
@@ -1,5 +1,5 @@
 : "${RUNTIME_arch_run?}"
-: ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
+: ${MAX_SMP:=$(getconf _NPROCESSORS_ONLN)}

I tested it on my machine by offlining a CPU and calling getconf _NPROCESSORS_CONF (returned 32) and getconf _NPROCESSORS_ONLN (returned 31). man 3 sysconf also agrees with your patch.

Quoted 3 lines

I am wondering though, if _NPROCESSORS_CONF is 8 and _NPROCESSORS_ONLN is 1 (meaning that 7 CPUs were offlined), that means that qemu will create 8 VCPUs which will share the same physical CPU. Is that undesirable?

With KVM enabled that's not recommended. KVM_CAP_NR_VCPUS returns the number of online VCPUs (at least for arm). Since the guest code may not run as expected with overcommitted VCPUs we don't usually want to test that way. OTOH, maybe we should write a test or two that does run with overcommitted VCPUs in order to look for bugs in KVM.

Thanks,
drew

Quoted 7 lines

Thanks,
Alex

Quoted 3 lines
scripts/runtime.bash +0 -0
 : ${TIMEOUT:=90s}
 
 PASS() { echo -ne "\e[32mPASS\e[0m"; }
#3 lore raw

Hi,

Quoted 24 lines · Andrew Jones
Quoted 23 lines · Alexandru Elisei

Hi,

Quoted 13 lines · Andrew Jones

We can only use online cpus, so make sure we check specifically for those.

Signed-off-by Andrew Jones <[email protected]>
---
 scripts/runtime.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/runtime.bash +0 -0
index 200d5b67290c..fbad0bd05fc5 100644

scripts/runtime.bash line 2

Quoted 3 lines
scripts/runtime.bash +1 -1
@@ -1,5 +1,5 @@
 : "${RUNTIME_arch_run?}"
-: ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
+: ${MAX_SMP:=$(getconf _NPROCESSORS_ONLN)}

I tested it on my machine by offlining a CPU and calling getconf _NPROCESSORS_CONF (returned 32) and getconf _NPROCESSORS_ONLN (returned 31). man 3 sysconf also agrees with your patch.

Quoted 6 lines
Quoted 3 lines

I am wondering though, if _NPROCESSORS_CONF is 8 and _NPROCESSORS_ONLN is 1 (meaning that 7 CPUs were offlined), that means that qemu will create 8 VCPUs which will share the same physical CPU. Is that undesirable?

With KVM enabled that's not recommended. KVM_CAP_NR_VCPUS returns the number of online VCPUs (at least for arm). Since the guest code may not run as expected with overcommitted VCPUs we don't usually want to test

Can you give more details about what may go wrong if several kvm-unit-tests VCPUs run on the same physical CPU? I was thinking that maybe we want to run the VCPUs in parallel (each on its own physical CPU) to detect races in KVM, not because of a limitation in kvm-unit-tests.

Thanks,
Alex

Quoted 10 lines

that way. OTOH, maybe we should write a test or two that does run with overcommitted VCPUs in order to look for bugs in KVM.

Thanks,
drew

Quoted 5 lines

Thanks,
Alex

Quoted 3 lines
scripts/runtime.bash +0 -0
 : ${TIMEOUT:=90s}
 
 PASS() { echo -ne "\e[32mPASS\e[0m"; }
author
#4 lore raw
Quoted 30 lines · Alexandru Elisei

Hi,

Quoted 27 lines · Andrew Jones
Quoted 26 lines · Alexandru Elisei

Hi,

Quoted 13 lines · Andrew Jones

We can only use online cpus, so make sure we check specifically for those.

Signed-off-by Andrew Jones <[email protected]>
---
 scripts/runtime.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/runtime.bash +0 -0
index 200d5b67290c..fbad0bd05fc5 100644

scripts/runtime.bash line 2

Quoted 3 lines
scripts/runtime.bash +1 -1
@@ -1,5 +1,5 @@
 : "${RUNTIME_arch_run?}"
-: ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
+: ${MAX_SMP:=$(getconf _NPROCESSORS_ONLN)}

I tested it on my machine by offlining a CPU and calling getconf _NPROCESSORS_CONF (returned 32) and getconf _NPROCESSORS_ONLN (returned 31). man 3 sysconf also agrees with your patch.

I am wondering though, if _NPROCESSORS_CONF is 8 and _NPROCESSORS_ONLN is 1 (meaning that 7 CPUs were offlined), that means that qemu will create 8 VCPUs which will share the same physical CPU. Is that undesirable?

Quoted 8 lines
Quoted 3 lines

With KVM enabled that's not recommended. KVM_CAP_NR_VCPUS returns the number of online VCPUs (at least for arm). Since the guest code may not run as expected with overcommitted VCPUs we don't usually want to test

Can you give more details about what may go wrong if several kvm-unit-tests VCPUs run on the same physical CPU? I was thinking that maybe we want to run the VCPUs in parallel (each on its own physical CPU) to detect races in KVM, not because of a limitation in kvm-unit-tests.

There's no specific limitation in kvm-unit-tests. All guest kernels are at risk of behaving strangely with overcommitted VCPUs.

A made-up example could be that a guest kernel thread T1 needs to wait for another thread T2. T1 may choose not to yield because T2 appears to be running on another CPU. But it isn't, because both T1's VCPU and T2's VCPU are running on the same PCPU. In general, telling a guest kernel it can run threads in parallel may cause it to make decisions that won't work well when executed serially.

In kvm-unit-tests, where we can know that we're overcommitted in certain test cases, we can actually write tests that will still behave predictably. Knowing the test runs fine with overcommitted VCPUs will allow us to check if KVM does as well.

Thanks,
drew

Quoted 14 lines

Thanks,
Alex

Quoted 10 lines

that way. OTOH, maybe we should write a test or two that does run with overcommitted VCPUs in order to look for bugs in KVM.

Thanks,
drew

Quoted 5 lines

Thanks,
Alex

Quoted 3 lines
scripts/runtime.bash +0 -0
 : ${TIMEOUT:=90s}
 
 PASS() { echo -ne "\e[32mPASS\e[0m"; }
Reviewed-by
#5 lore raw

Hi,

Quoted 48 lines · Andrew Jones
Quoted 30 lines · Alexandru Elisei

Hi,

Quoted 27 lines · Andrew Jones
Quoted 26 lines · Alexandru Elisei

Hi,

Quoted 13 lines · Andrew Jones

We can only use online cpus, so make sure we check specifically for those.

Signed-off-by Andrew Jones <[email protected]>
---
 scripts/runtime.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/runtime.bash +0 -0
index 200d5b67290c..fbad0bd05fc5 100644

scripts/runtime.bash line 2

Quoted 3 lines
scripts/runtime.bash +1 -1
@@ -1,5 +1,5 @@
 : "${RUNTIME_arch_run?}"
-: ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
+: ${MAX_SMP:=$(getconf _NPROCESSORS_ONLN)}

I tested it on my machine by offlining a CPU and calling getconf _NPROCESSORS_CONF (returned 32) and getconf _NPROCESSORS_ONLN (returned 31). man 3 sysconf also agrees with your patch.

I am wondering though, if _NPROCESSORS_CONF is 8 and _NPROCESSORS_ONLN is 1 (meaning that 7 CPUs were offlined), that means that qemu will create 8 VCPUs which will share the same physical CPU. Is that undesirable?

Quoted 7 lines
Quoted 3 lines

With KVM enabled that's not recommended. KVM_CAP_NR_VCPUS returns the number of online VCPUs (at least for arm). Since the guest code may not run as expected with overcommitted VCPUs we don't usually want to test

Can you give more details about what may go wrong if several kvm-unit-tests VCPUs run on the same physical CPU? I was thinking that maybe we want to run the VCPUs in parallel (each on its own physical CPU) to detect races in KVM, not because of a limitation in kvm-unit-tests.

There's no specific limitation in kvm-unit-tests. All guest kernels are at risk of behaving strangely with overcommitted VCPUs.

A made-up example could be that a guest kernel thread T1 needs to wait for another thread T2. T1 may choose not to yield because T2 appears to be running on another CPU. But it isn't, because both T1's VCPU and T2's VCPU are running on the same PCPU. In general, telling a guest kernel it can run threads in parallel may cause it to make decisions that won't work well when executed serially.

Quoted 4 lines

In kvm-unit-tests, where we can know that we're overcommitted in certain test cases, we can actually write tests that will still behave predictably. Knowing the test runs fine with overcommitted VCPUs will allow us to check if KVM does as well.

Makes sense, thank you for the explanation. For the patch:

Reviewed-by Alexandru Elisei <[email protected]>

Thanks,
Alex

Quoted 15 lines

Thanks,
drew

Quoted 12 lines

Thanks,
Alex

Quoted 10 lines

that way. OTOH, maybe we should write a test or two that does run with overcommitted VCPUs in order to look for bugs in KVM.

Thanks,
drew

Quoted 5 lines

Thanks,
Alex

Quoted 3 lines
scripts/runtime.bash +0 -0
 : ${TIMEOUT:=90s}
 
 PASS() { echo -ne "\e[32mPASS\e[0m"; }
maintainer Applied
#6 lore raw
In reply to Andrew Jones
Quoted 14 lines · Andrew Jones

We can only use online cpus, so make sure we check specifically for those.

Signed-off-by Andrew Jones <[email protected]>
---
 scripts/runtime.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/runtime.bash +0 -0
index 200d5b67290c..fbad0bd05fc5 100644

scripts/runtime.bash line 5

Quoted 6 lines
scripts/runtime.bash +1 -1
@@ -1,5 +1,5 @@
 : "${RUNTIME_arch_run?}"
-: ${MAX_SMP:=$(getconf _NPROCESSORS_CONF)}
+: ${MAX_SMP:=$(getconf _NPROCESSORS_ONLN)}
 : ${TIMEOUT:=90s}
 
 PASS() { echo -ne "\e[32mPASS\e[0m"; }

Applied, thanks.

Paolo