Quoting Nina Schoetterl-Glausch (2022-12-12 21:37:28)
> 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?
It is a good point, actually I just did what we already do in wait_for_flag in s390x/smp.c. :-)
> 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).
I agree, but it does not matter, does it? At latest the second iteration will actually read from memory, no?
> In any case using a do while loop instead would eliminate the question.
> A comment might be nice, too.
How about I change to
while(!READ_ONCE(thread_exited));
and add an explicit mb() below to ensure result is read from memory?
