On Fri, 2022-12-09 at 11:21 +0100, Nico Boehr wrote:
> Right now, we have a test which sets storage keys, then migrates the VM
> and - after migration finished - verifies the skeys are still there.
>
> Add a new version of the test which changes storage keys while the
> migration is in progress. This is achieved by adding a command line
> argument to the existing migration-skey test.
>
> Signed-off-by: Nico Boehr <nrb@xxxxxxxxxxxxx>
> ---
> s390x/migration-skey.c | 214 +++++++++++++++++++++++++++++++++++------
> s390x/unittests.cfg | 15 ++-
> 2 files changed, 198 insertions(+), 31 deletions(-)
>
> diff --git a/s390x/migration-skey.c b/s390x/migration-skey.c
> index b7bd82581abe..9b9a45f4ad3b 100644
> --- a/s390x/migration-skey.c
> +++ b/s390x/migration-skey.c
>
[...]
> +static void test_skey_migration_parallel(void)
> +{
> + report_prefix_push("parallel");
> +
> + if (smp_query_num_cpus() == 1) {
> + report_skip("need at least 2 cpus for this test");
> + goto error;
> + }
> +
> + smp_cpu_setup(1, PSW_WITH_CUR_MASK(set_skeys_thread));
> +
> + migrate_once();
> +
> + WRITE_ONCE(thread_should_exit, 1);
> +
> + while (!thread_exited)
> + mb();
Are you doing it this way instead of while(!READ_ONCE(thread_exited)); so the mb() does double duty
and ensures "result" is also read from memory a couple of lines down?
If so, I wonder if the compiler is allowed to arrange the control flow such that if the loop condition
is false on the first iteration it uses a cached value of "result" (I'd be guessing yes, but what do I know).
In any case using a do while loop instead would eliminate the question.
A comment might be nice, too.
> +
> + report_info("thread completed %u iterations", thread_iters);
> +
> + report_prefix_push("during migration");
> + skey_report_verify(&result);
> + report_prefix_pop();
> +
> + /*
> + * Verification of skeys occurs on the thread. We don't know if we
> + * were still migrating during the verification.
> + * To be sure, make another verification round after the migration
> + * finished to catch skeys which might not have been migrated
> + * correctly.
> + */
> + report_prefix_push("after migration");
> + assert(thread_iters > 0);
> + result = skey_verify_test_pattern(pagebuf, NUM_PAGES, thread_iters - 1);
> + skey_report_verify(&result);
> + report_prefix_pop();
> +
> +error:
> + report_prefix_pop();
> +}
> +
[...]
