Re: [kvm-unit-tests PATCH] arm: Remove MAX_SMP probe loop
On Thu, Jan 05, 2023 at 11:09:58PM +, Colton Lewis wrote: > Andrew Jones writes: > > On Tue, Dec 20, 2022 at 04:32:00PM +, Colton Lewis wrote: > > > Alexandru Elisei writes: > > Ah, I think I understand now. Were you running 32-bit arm tests? If so, > > it'd be good to point that out explicitly in the commit message (the > > 'arm:' prefix in the summary is ambiguous). > > No, this was happening on arm64. Since it had been a while since I noted > this issue, I reviewed it and realized the issue was only happening > using -accel tcg. That was automatically being used on my problem test > machine without me noticing. That's where the limit of 8 seems to be > coming from and why the loop is triggered. > > qemu-system-aarch64: Number of SMP CPUs requested (152) exceeds max CPUs > supported by machine 'mach-virt' (8) > > Since this case doesn't directly involve KVM, I doubt anyone cares about > a fix. > > > Assuming the loop body was running because it needed to reduce MAX_SMP to > > 8 or lower for 32-bit arm tests, then we should be replacing the loop with > > something that caps MAX_SMP at 8 for 32-bit arm tests instead. > > We could cap at 8 for ACCEL=tcg. Even if no one cares, I'm tempted to do > it so no one hits the same little landmine as me in the future. TCG supports up to 255 CPUs. The only reason it'd have a max of 8 is if you were configuring a GICv2 instead of a GICv3. Using gic-version=3 or gic-version=max should allow the 152 CPUs to work. Actually, I should have asked about your gic version instead of whether or not the VM was AArch32 in the first place. I was incorrectly associating the gicv2 limits with arm32 since my memories of these things have started to blur together... Thanks, drew ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH] arm: Remove MAX_SMP probe loop
Andrew Jones writes: On Tue, Dec 20, 2022 at 04:32:00PM +, Colton Lewis wrote: Alexandru Elisei writes: Ah, I think I understand now. Were you running 32-bit arm tests? If so, it'd be good to point that out explicitly in the commit message (the 'arm:' prefix in the summary is ambiguous). No, this was happening on arm64. Since it had been a while since I noted this issue, I reviewed it and realized the issue was only happening using -accel tcg. That was automatically being used on my problem test machine without me noticing. That's where the limit of 8 seems to be coming from and why the loop is triggered. qemu-system-aarch64: Number of SMP CPUs requested (152) exceeds max CPUs supported by machine 'mach-virt' (8) Since this case doesn't directly involve KVM, I doubt anyone cares about a fix. Assuming the loop body was running because it needed to reduce MAX_SMP to 8 or lower for 32-bit arm tests, then we should be replacing the loop with something that caps MAX_SMP at 8 for 32-bit arm tests instead. We could cap at 8 for ACCEL=tcg. Even if no one cares, I'm tempted to do it so no one hits the same little landmine as me in the future. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH] arm: Remove MAX_SMP probe loop
On Tue, Dec 20, 2022 at 04:32:00PM +, Colton Lewis wrote: > Alexandru Elisei writes: > > > Though I'm not sure how you managed to get MAX_SMP to go down to 6 cores > > on > > a 12 core machine. MAX_SMP is initialized to $(getconf _NPROCESSORS_ONLN), > > so the body of the loop should never execute. I also tried it on a 6 core > > machine, and MAX_SMP was 6, not 3. > > > Am I missing something? > > To be clear, 12 cores was a simplified example I did not directly > verify. What happened to me was 152 cores being cut down to 4. I was > confused why one machine was running a test with 4 cores when my other > machines were running with 8 and traced it to that loop. In effect the > loop was doing MAX_SMP=floor(MAX_SMP / 2) until MAX_SMP <= 8. I printed > the iterations and MAX_SMP followed the sequence 152->76->38->19->9->4. Ah, I think I understand now. Were you running 32-bit arm tests? If so, it'd be good to point that out explicitly in the commit message (the 'arm:' prefix in the summary is ambiguous). Assuming the loop body was running because it needed to reduce MAX_SMP to 8 or lower for 32-bit arm tests, then we should be replacing the loop with something that caps MAX_SMP at 8 for 32-bit arm tests instead. Thanks, drew ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH] arm: Remove MAX_SMP probe loop
On Mon, Dec 19, 2022 at 06:52:50PM +, Colton Lewis wrote: > This loop logic is broken for machines with a number of CPUs that > isn't a power of two. A machine with 8 CPUs will test with MAX_SMP=8 > but a machine with 12 CPUs will test with MAX_SMP=6 because 12 >> 2 == ^ 1 > 6. This can, in rare circumstances, lead to different test results > depending only on the number of CPUs the machine has. > > The loop is safe to remove with no side effects. It has an explanitory > comment explaining that it only applies to kernels <=v4.3 on arm and > suggestion deletion when it becomes tiresome to maintain. Removing this loop is safe if the body of the loop is not expected to ever run, i.e. we're through testing kernels older than v4.3. But, if MAX_SMP is getting reduced, as stated above, then that implies $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP is resulting in a "exceeds max CPUs" error. If that's the case, then the tests which configure $MAX_SMP cpus should be failing as well. IOW, the loop should never do anything except be a pointless extra invocation of QEMU. If it does do something, then we should understand why. Thanks, drew > > Signed-off-by: Colton Lewis > --- > scripts/runtime.bash | 14 -- > 1 file changed, 14 deletions(-) > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > index f8794e9..18a8dd7 100644 > --- a/scripts/runtime.bash > +++ b/scripts/runtime.bash > @@ -183,17 +183,3 @@ function run() > > return $ret > } > - > -# > -# Probe for MAX_SMP, in case it's less than the number of host cpus. > -# > -# This probing currently only works for ARM, as x86 bails on another > -# error first. Also, this probing isn't necessary for any ARM hosts > -# running kernels later than v4.3, i.e. those including ef748917b52 > -# "arm/arm64: KVM: Remove 'config KVM_ARM_MAX_VCPUS'". So, at some > -# point when maintaining the while loop gets too tiresome, we can > -# just remove it... > -while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \ > - |& grep -qi 'exceeds max CPUs'; do > - MAX_SMP=$((MAX_SMP >> 1)) > -done > -- > 2.39.0.314.g84b9a713c41-goog > ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH] arm: Remove MAX_SMP probe loop
Alexandru Elisei writes: Though I'm not sure how you managed to get MAX_SMP to go down to 6 cores on a 12 core machine. MAX_SMP is initialized to $(getconf _NPROCESSORS_ONLN), so the body of the loop should never execute. I also tried it on a 6 core machine, and MAX_SMP was 6, not 3. Am I missing something? To be clear, 12 cores was a simplified example I did not directly verify. What happened to me was 152 cores being cut down to 4. I was confused why one machine was running a test with 4 cores when my other machines were running with 8 and traced it to that loop. In effect the loop was doing MAX_SMP=floor(MAX_SMP / 2) until MAX_SMP <= 8. I printed the iterations and MAX_SMP followed the sequence 152->76->38->19->9->4. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH] arm: Remove MAX_SMP probe loop
Hi, On Mon, Dec 19, 2022 at 06:52:50PM +, Colton Lewis wrote: > This loop logic is broken for machines with a number of CPUs that > isn't a power of two. A machine with 8 CPUs will test with MAX_SMP=8 > but a machine with 12 CPUs will test with MAX_SMP=6 because 12 >> 2 == > 6. This can, in rare circumstances, lead to different test results > depending only on the number of CPUs the machine has. I do think removing the loop is a good idea. I tested this patch, and when running a single test, it makes the run 3 seconds faster on an Cortex-A53 for me - selftest-smp and selftest-vectors-kernel went from taking 12s to 9s. Doesn't make much of a difference when running all the tests (those take for me 5m10s without the patch, 5m6.5s with the patch), but when running a single test the 25% speed improvement is noticeable. Though I'm not sure how you managed to get MAX_SMP to go down to 6 cores on a 12 core machine. MAX_SMP is initialized to $(getconf _NPROCESSORS_ONLN), so the body of the loop should never execute. I also tried it on a 6 core machine, and MAX_SMP was 6, not 3. Am I missing something? Thanks, Alex > > The loop is safe to remove with no side effects. It has an explanitory > comment explaining that it only applies to kernels <=v4.3 on arm and > suggestion deletion when it becomes tiresome to maintain. > > Signed-off-by: Colton Lewis > --- > scripts/runtime.bash | 14 -- > 1 file changed, 14 deletions(-) > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > index f8794e9..18a8dd7 100644 > --- a/scripts/runtime.bash > +++ b/scripts/runtime.bash > @@ -183,17 +183,3 @@ function run() > > return $ret > } > - > -# > -# Probe for MAX_SMP, in case it's less than the number of host cpus. > -# > -# This probing currently only works for ARM, as x86 bails on another > -# error first. Also, this probing isn't necessary for any ARM hosts > -# running kernels later than v4.3, i.e. those including ef748917b52 > -# "arm/arm64: KVM: Remove 'config KVM_ARM_MAX_VCPUS'". So, at some > -# point when maintaining the while loop gets too tiresome, we can > -# just remove it... > -while $RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP \ > - |& grep -qi 'exceeds max CPUs'; do > - MAX_SMP=$((MAX_SMP >> 1)) > -done > -- > 2.39.0.314.g84b9a713c41-goog > ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
