* [PATCH] automation: fix race condition in adl-suspend test @ 2023-10-28 3:33 Marek Marczykowski-Górecki 2023-10-30 11:42 ` Roger Pau Monné 0 siblings, 1 reply; 4+ messages in thread From: Marek Marczykowski-Górecki @ 2023-10-28 3:33 UTC (permalink / raw) To: xen-devel Cc: Marek Marczykowski-Górecki, Doug Goldstein, Stefano Stabellini If system suspends too quickly, the message for the test controller to wake up the system may be not sent to the console before suspending. This will cause the test to timeout. Fix this by waiting a bit after printing the message. The test controller then resumes the system 30s after the message, so as long as the delay + suspending takes less time it is okay. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- This is consistent with the observation that sync_console "fixes" the issue. --- automation/scripts/qubes-x86-64.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh index 26131b082671..a34db96e4585 100755 --- a/automation/scripts/qubes-x86-64.sh +++ b/automation/scripts/qubes-x86-64.sh @@ -54,11 +54,11 @@ until grep 'domU started' /var/log/xen/console/guest-domU.log; do sleep 1 done echo \"${wait_and_wakeup}\" +# let the above message flow to console, then suspend +sleep 5 set -x echo deep > /sys/power/mem_sleep echo mem > /sys/power/state -# now wait for resume -sleep 5 xl list xl dmesg | grep 'Finishing wakeup from ACPI S3 state' || exit 1 # check if domU is still alive -- 2.41.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] automation: fix race condition in adl-suspend test 2023-10-28 3:33 [PATCH] automation: fix race condition in adl-suspend test Marek Marczykowski-Górecki @ 2023-10-30 11:42 ` Roger Pau Monné 2023-10-30 16:32 ` Marek Marczykowski-Górecki 0 siblings, 1 reply; 4+ messages in thread From: Roger Pau Monné @ 2023-10-30 11:42 UTC (permalink / raw) To: Marek Marczykowski-Górecki Cc: xen-devel, Doug Goldstein, Stefano Stabellini On Sat, Oct 28, 2023 at 05:33:57AM +0200, Marek Marczykowski-Górecki wrote: > If system suspends too quickly, the message for the test controller to > wake up the system may be not sent to the console before suspending. > This will cause the test to timeout. > > Fix this by waiting a bit after printing the message. The test > controller then resumes the system 30s after the message, so as long as > the delay + suspending takes less time it is okay. > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > --- > This is consistent with the observation that sync_console "fixes" the > issue. > --- > automation/scripts/qubes-x86-64.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh > index 26131b082671..a34db96e4585 100755 > --- a/automation/scripts/qubes-x86-64.sh > +++ b/automation/scripts/qubes-x86-64.sh > @@ -54,11 +54,11 @@ until grep 'domU started' /var/log/xen/console/guest-domU.log; do > sleep 1 > done > echo \"${wait_and_wakeup}\" > +# let the above message flow to console, then suspend > +sleep 5 Could you use `sync /dev/stdout`? I guess that might not be enough, since the sync won't be propagated to the hypervisor, and hence even if flushed from Linux, we have no guarantee that the hypervisor has also flushed it. Xen should flush the buffer when a newline character is found, but I have no idea whether context could return to guest while the buffer is still in the process of being fully flushed. Anyway, adding the extra sync might be good regardless, and keeping the sleep. Thanks, Roger. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] automation: fix race condition in adl-suspend test 2023-10-30 11:42 ` Roger Pau Monné @ 2023-10-30 16:32 ` Marek Marczykowski-Górecki 2023-10-30 16:38 ` Marek Marczykowski-Górecki 0 siblings, 1 reply; 4+ messages in thread From: Marek Marczykowski-Górecki @ 2023-10-30 16:32 UTC (permalink / raw) To: Roger Pau Monné; +Cc: xen-devel, Doug Goldstein, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 2228 bytes --] On Mon, Oct 30, 2023 at 12:42:52PM +0100, Roger Pau Monné wrote: > On Sat, Oct 28, 2023 at 05:33:57AM +0200, Marek Marczykowski-Górecki wrote: > > If system suspends too quickly, the message for the test controller to > > wake up the system may be not sent to the console before suspending. > > This will cause the test to timeout. > > > > Fix this by waiting a bit after printing the message. The test > > controller then resumes the system 30s after the message, so as long as > > the delay + suspending takes less time it is okay. > > > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > --- > > This is consistent with the observation that sync_console "fixes" the > > issue. > > --- > > automation/scripts/qubes-x86-64.sh | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh > > index 26131b082671..a34db96e4585 100755 > > --- a/automation/scripts/qubes-x86-64.sh > > +++ b/automation/scripts/qubes-x86-64.sh > > @@ -54,11 +54,11 @@ until grep 'domU started' /var/log/xen/console/guest-domU.log; do > > sleep 1 > > done > > echo \"${wait_and_wakeup}\" > > +# let the above message flow to console, then suspend > > +sleep 5 > > Could you use `sync /dev/stdout`? I guess that might not be enough, > since the sync won't be propagated to the hypervisor, and hence even > if flushed from Linux, we have no guarantee that the hypervisor has > also flushed it. It seems `sync /dev/stdout` helps too, at least in a limited sample of two. > Xen should flush the buffer when a newline character is found, but I > have no idea whether context could return to guest while the buffer is > still in the process of being fully flushed. IIC Xen should flush the console buffer on the suspend path (there is console_start_sync() in enter_state()). So, if linux manages to send it to Xen in time, all should be good (in theory at least). > Anyway, adding the extra sync might be good regardless, and keeping > the sleep. Good idea, I'll send v2 with it included. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] automation: fix race condition in adl-suspend test 2023-10-30 16:32 ` Marek Marczykowski-Górecki @ 2023-10-30 16:38 ` Marek Marczykowski-Górecki 0 siblings, 0 replies; 4+ messages in thread From: Marek Marczykowski-Górecki @ 2023-10-30 16:38 UTC (permalink / raw) To: Roger Pau Monné; +Cc: xen-devel, Doug Goldstein, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 2470 bytes --] On Mon, Oct 30, 2023 at 05:32:08PM +0100, Marek Marczykowski-Górecki wrote: > On Mon, Oct 30, 2023 at 12:42:52PM +0100, Roger Pau Monné wrote: > > On Sat, Oct 28, 2023 at 05:33:57AM +0200, Marek Marczykowski-Górecki wrote: > > > If system suspends too quickly, the message for the test controller to > > > wake up the system may be not sent to the console before suspending. > > > This will cause the test to timeout. > > > > > > Fix this by waiting a bit after printing the message. The test > > > controller then resumes the system 30s after the message, so as long as > > > the delay + suspending takes less time it is okay. > > > > > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > > --- > > > This is consistent with the observation that sync_console "fixes" the > > > issue. > > > --- > > > automation/scripts/qubes-x86-64.sh | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh > > > index 26131b082671..a34db96e4585 100755 > > > --- a/automation/scripts/qubes-x86-64.sh > > > +++ b/automation/scripts/qubes-x86-64.sh > > > @@ -54,11 +54,11 @@ until grep 'domU started' /var/log/xen/console/guest-domU.log; do > > > sleep 1 > > > done > > > echo \"${wait_and_wakeup}\" > > > +# let the above message flow to console, then suspend > > > +sleep 5 > > > > Could you use `sync /dev/stdout`? I guess that might not be enough, > > since the sync won't be propagated to the hypervisor, and hence even > > if flushed from Linux, we have no guarantee that the hypervisor has > > also flushed it. > > It seems `sync /dev/stdout` helps too, at least in a limited sample of > two. ... and the third attempt (with sync instead of sleep) failed. > > Xen should flush the buffer when a newline character is found, but I > > have no idea whether context could return to guest while the buffer is > > still in the process of being fully flushed. > > IIC Xen should flush the console buffer on the suspend path (there is > console_start_sync() in enter_state()). So, if linux manages to send it > to Xen in time, all should be good (in theory at least). > > > Anyway, adding the extra sync might be good regardless, and keeping > > the sleep. > > Good idea, I'll send v2 with it included. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-30 16:39 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-28 3:33 [PATCH] automation: fix race condition in adl-suspend test Marek Marczykowski-Górecki 2023-10-30 11:42 ` Roger Pau Monné 2023-10-30 16:32 ` Marek Marczykowski-Górecki 2023-10-30 16:38 ` Marek Marczykowski-Górecki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.