Re: [PATCH v21 15/20] tests/avocado: s390x cpu topology polarisation

[Pierre Morel]

On 7/5/23 10:53, Thomas Huth wrote:
> On 30/06/2023 11.17, Pierre Morel wrote:
> > Polarization is changed on a request from the guest.
> > Let's verify the polarization is accordingly set by QEMU.
> >
> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > ---
> >   tests/avocado/s390_topology.py | 46 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 46 insertions(+)
> >
> > diff --git a/tests/avocado/s390_topology.py 
> > b/tests/avocado/s390_topology.py
> > index 1758ec1f13..2cf731cb1d 100644
> > --- a/tests/avocado/s390_topology.py
> > +++ b/tests/avocado/s390_topology.py
> > @@ -40,6 +40,7 @@ class S390CPUTopology(QemuSystemTest):
> >       The polarization is changed on a request from the guest.
> >       """
> >       timeout = 90
> > +    event_timeout = 1
>
> When running tests in CI and the machines are very loaded, the tests 
> can be stalled easily by multiple seconds. So using a timeout of 1 
> seconds sounds way too low for me. Please use at least 5 seconds, or 
> maybe even 10.


OK


> >       KERNEL_COMMON_COMMAND_LINE = ('printk.time=0 '
> >                                     'root=/dev/ram '
> > @@ -99,6 +100,15 @@ def kernel_init(self):
> >                            '-initrd', initrd_path,
> >                            '-append', kernel_command_line)
> >   +    def system_init(self):
> > +        self.log.info("System init")
> > +        exec_command(self, 'mount proc -t proc /proc')
> > +        time.sleep(0.2)
> > +        exec_command(self, 'mount sys -t sysfs /sys')
> > +        time.sleep(0.2)
>
> Hard coded sleeps are ugly... they are prone to race conditions (e.g. 
> on loaded test systems), and they artificially slow down the test 
> duration.
>
> What about doing all three commands in one statement instead:
>
>     exec_command_and_wait_for_pattern(self,
>            """mount proc -t proc /proc ;
>               mount sys -t sysfs /sys ;
>               /bin/cat /sys/devices/system/cpu/dispatching""",
>            '0')
>
> ?
OK , I use this. thx.

Regards,

Pierre