On Mon Mar 4, 2024 at 4:17 PM AEST, Thomas Huth wrote:
> On 26/02/2024 10.38, Nicholas Piggin wrote:
> > The cooperative migration protocol is very good to control precise
> > pre and post conditions for a migration event. However in some cases
> > its intrusiveness to the test program, can mask problems and make
> > analysis more difficult.
> >
> > For example to stress test migration vs concurrent complicated
> > memory access, including TLB refill, ram dirtying, etc., then the
> > tight spin at getchar() and resumption of the workload after
> > migration is unhelpful.
> >
> > This adds a continuous migration mode that directs the harness to
> > perform migrations continually. This is added to the migration
> > selftests, which also sees cooperative migration iterations reduced
> > to avoid increasing test time too much.
> >
> > Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> > ---
> > common/selftest-migration.c | 16 +++++++++--
> > lib/migrate.c | 18 ++++++++++++
> > lib/migrate.h | 3 ++
> > scripts/arch-run.bash | 55 ++++++++++++++++++++++++++++++++-----
> > 4 files changed, 82 insertions(+), 10 deletions(-)
> >
> > diff --git a/common/selftest-migration.c b/common/selftest-migration.c
> > index 0afd8581c..9a9b61835 100644
> > --- a/common/selftest-migration.c
> > +++ b/common/selftest-migration.c
> > @@ -9,12 +9,13 @@
> > */
> > #include <libcflat.h>
> > #include <migrate.h>
> > +#include <asm/time.h>
> >
> > -#define NR_MIGRATIONS 30
> > +#define NR_MIGRATIONS 15
> >
> > int main(int argc, char **argv)
> > {
> > - report_prefix_push("migration");
> > + report_prefix_push("migration harness");
> >
> > if (argc > 1 && !strcmp(argv[1], "skip")) {
> > migrate_skip();
> > @@ -24,7 +25,16 @@ int main(int argc, char **argv)
> >
> > for (i = 0; i < NR_MIGRATIONS; i++)
> > migrate_quiet();
> > - report(true, "simple harness stress");
> > + report(true, "cooperative migration");
> > +
> > + migrate_begin_continuous();
> > + mdelay(2000);
> > + migrate_end_continuous();
> > + mdelay(1000);
> > + migrate_begin_continuous();
> > + mdelay(2000);
> > + migrate_end_continuous();
> > + report(true, "continuous migration");
> > }
> >
> > report_prefix_pop();
> > diff --git a/lib/migrate.c b/lib/migrate.c
> > index 1d22196b7..770f76d5c 100644
> > --- a/lib/migrate.c
> > +++ b/lib/migrate.c
> > @@ -60,3 +60,21 @@ void migrate_skip(void)
> > puts("Skipped VM migration (quiet)\n");
> > (void)getchar();
> > }
> > +
> > +void migrate_begin_continuous(void)
> > +{
> > + puts("Begin continuous migration\n");
> > + (void)getchar();
> > +}
> > +
> > +void migrate_end_continuous(void)
> > +{
> > + /*
> > + * Migration can split this output between source and dest QEMU
> > + * output files, print twice and match once to always cope with
> > + * a split.
> > + */
> > + puts("End continuous migration\n");
> > + puts("End continuous migration (quiet)\n");
> > + (void)getchar();
> > +}
> > diff --git a/lib/migrate.h b/lib/migrate.h
> > index db6e0c501..35b6703a2 100644
> > --- a/lib/migrate.h
> > +++ b/lib/migrate.h
> > @@ -11,3 +11,6 @@ void migrate_quiet(void);
> > void migrate_once(void);
> >
> > void migrate_skip(void);
> > +
> > +void migrate_begin_continuous(void);
> > +void migrate_end_continuous(void);
> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > index d0f6f098f..5c7e72036 100644
> > --- a/scripts/arch-run.bash
> > +++ b/scripts/arch-run.bash
> > @@ -125,15 +125,17 @@ qmp_events ()
> > filter_quiet_msgs ()
> > {
> > grep -v "Now migrate the VM (quiet)" |
> > + grep -v "Begin continuous migration (quiet)" |
> > + grep -v "End continuous migration (quiet)" |
> > grep -v "Skipped VM migration (quiet)"
> > }
> >
> > seen_migrate_msg ()
> > {
> > if [ $skip_migration -eq 1 ]; then
> > - grep -q -e "Now migrate the VM" < $1
> > + grep -q -e "Now migrate the VM" -e "Begin continuous migration"
> > < $1
> > else
> > - grep -q -e "Now migrate the VM" -e "Skipped VM migration" < $1
> > + grep -q -e "Now migrate the VM" -e "Begin continuous migration"
> > -e "Skipped VM migration" < $1
> > fi
> > }
> >
> > @@ -161,6 +163,7 @@ run_migration ()
> > src_qmpout=/dev/null
> > dst_qmpout=/dev/null
> > skip_migration=0
> > + continuous_migration=0
> >
> > mkfifo ${src_outfifo}
> > mkfifo ${dst_outfifo}
> > @@ -186,9 +189,12 @@ run_migration ()
> > do_migration || return $?
> >
> > while ps -p ${live_pid} > /dev/null ; do
> > - # Wait for test exit or further migration messages.
> > - if ! seen_migrate_msg ${src_out} ; then
> > + if [[ ${continuous_migration} -eq 1 ]] ; then
>
> Here you're using "[[" for testing ...
>
> > + do_migration || return $?
> > + elif ! seen_migrate_msg ${src_out} ; then
> > sleep 0.1
> > + elif grep -q "Begin continuous migration" < ${src_out} ; then
> > + do_migration || return $?
> > elif grep -q "Now migrate the VM" < ${src_out} ; then
> > do_migration || return $?
> > elif [ $skip_migration -eq 0 ] && grep -q "Skipped VM
> > migration" < ${src_out} ; then
>
> ... while the other code seems to use "[" for testing values. Can we try to
> stick to one style, please (unless it's really required to use "[["
> somewhere)?
Good point. Will do.
Thanks,
Nick
