On Tue, Jan 31, 2023 at 08:41:39AM +0100, Thomas Huth wrote: > On 31/01/2023 07.32, Andrew Jones wrote: > > On Mon, Jan 30, 2023 at 07:57:00PM +0000, Colton Lewis wrote: > > > Replace the MAX_SMP probe loop in favor of reading a number directly > > > from the QEMU error message. This is equally safe as the existing code > > > because the error message has had the same format as long as it has > > > existed, since QEMU v2.10. The final number before the end of the > > > error message line indicates the max QEMU supports. A short awk > > > > awk is not used, despite the comment also being updated to say it's > > being used. > > > > > program is used to extract the number, which becomes the new MAX_SMP > > > value. > > > > > > This loop logic is broken for machines with a number of CPUs that > > > isn't a power of two. This problem was noticed for gicv2 tests on > > > machines with a non-power-of-two number of CPUs greater than 8 because > > > tests were running with MAX_SMP less than 8. As a hypthetical example, > > s/hypthetical/hypothetical/ > > > > a machine with 12 CPUs will test with MAX_SMP=6 because 12 >> 1 == > > > 6. This can, in rare circumstances, lead to different test results > > > depending only on the number of CPUs the machine has. > > > > > > A previous comment explains the loop should only apply to kernels > > > <=v4.3 on arm and suggests deletion when it becomes tiresome to > > > maintian. However, it is always theoretically possible to test on a > > s/maintian/maintain/ > > > > machine that has more CPUs than QEMU supports, so it makes sense to > > > leave some check in place. > > > > > > Signed-off-by: Colton Lewis <coltonlewis@xxxxxxxxxx> > > > --- > > > scripts/runtime.bash | 16 +++++++--------- > > > 1 file changed, 7 insertions(+), 9 deletions(-) > > > > > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > > > index f8794e9a..587ffe30 100644 > > > --- a/scripts/runtime.bash > > > +++ b/scripts/runtime.bash > > > @@ -188,12 +188,10 @@ function run() > > > # 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 > > > > It just occurred to me that this code runs on all architectures, even > > though it only works for Arm. We should wrap this code in $ARCH > > checks or put it in a function which only Arm calls. That change > > should be a separate patch though. > > Or we just grep for "max CPUs", since this seems to be used on other > architectures, too: > > $ qemu-system-x86_64 -smp 12345 > qemu-system-x86_64: Invalid SMP CPUs 12345. The max CPUs supported by > machine 'pc-i440fx-8.0' is 255 > > ? > Yes, if we can find an arch-common way to set MAX_SMP, then the variable could be used in their test configs and gitlab-ci scripts. For example, afaict, x86 doesn't have any tests that run with more than 4 cpus at the moment. Being able to bump that up for some tests might increase test coverage. That said, it might be stretching the scope of this patch a bit much. How about we keep the grep the same for now and guard with $ARCH. Other architectures can either share Arm's grep, tweak the grep and share it, or add their own grep, when/if they want to start using MAX_SMP. Thanks, drew
