Messages in this thread | | | Subject | Re: [PATCH v3] KVM: selftests: Fix target thread to be migrated in rseq_test | From | Gavin Shan <> | Date | Tue, 19 Jul 2022 12:13:32 +1000 |
| |
Hi Sean,
On 7/19/22 9:46 AM, Sean Christopherson wrote: > On Tue, Jul 19, 2022, Gavin Shan wrote: >> --- >> v3: Improved changelog (Oliver Upon) > > Sorry I didn't catch v3, I saw that you waited but just didn't get to this earlier :-/ >
Not a problem at all :)
>> --- >> tools/testing/selftests/kvm/rseq_test.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c >> index 4158da0da2bb..c83ac7b467f8 100644 >> --- a/tools/testing/selftests/kvm/rseq_test.c >> +++ b/tools/testing/selftests/kvm/rseq_test.c >> @@ -38,6 +38,7 @@ static __thread volatile struct rseq __rseq = { >> */ >> #define NR_TASK_MIGRATIONS 100000 >> >> +static pid_t rseq_tid; >> static pthread_t migration_thread; >> static cpu_set_t possible_mask; >> static int min_cpu, max_cpu; >> @@ -106,7 +107,8 @@ static void *migration_worker(void *ign) > > Pass the target TID to the worker, then there's no need to use a global and no > chance of consuming rseq_tid "uninitialized". The casting to convert gettid() to > a "void *" is annoying, but not the end of the world. >
I was thinking of the scheme, but passing the address of a local variable for the thread ID. Your suggestion also makes sense to me.
>> * stable, i.e. while changing affinity is in-progress. >> */ >> smp_wmb(); >> - r = sched_setaffinity(0, sizeof(allowed_mask), &allowed_mask); >> + r = sched_setaffinity(rseq_tid, sizeof(allowed_mask), >> + &allowed_mask); > > Eh, let this poke out, don't think it's worth wrapping here. >
Ok, I was trying to follow rule of 80-characters per line, even it's not strictly needed nowadays. It's also fine not to follow :)
I just picked your code and posted v4:
https://lore.kernel.org/kvmarm/20220719020830.3479482-1-gshan@redhat.com/T/#u
Thanks, Gavin
> E.g. > > --- > tools/testing/selftests/kvm/rseq_test.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c > index aba7be178dab..a54d4d05a058 100644 > --- a/tools/testing/selftests/kvm/rseq_test.c > +++ b/tools/testing/selftests/kvm/rseq_test.c > @@ -80,8 +80,9 @@ static int next_cpu(int cpu) > return cpu; > } > > -static void *migration_worker(void *ign) > +static void *migration_worker(void *__rseq_tid) > { > + pid_t rseq_tid = (pid_t)(unsigned long)__rseq_tid; > cpu_set_t allowed_mask; > int r, i, cpu; > > @@ -104,7 +105,7 @@ static void *migration_worker(void *ign) > * stable, i.e. while changing affinity is in-progress. > */ > smp_wmb(); > - r = sched_setaffinity(0, sizeof(allowed_mask), &allowed_mask); > + r = sched_setaffinity(rseq_tid, sizeof(allowed_mask), &allowed_mask); > TEST_ASSERT(!r, "sched_setaffinity failed, errno = %d (%s)", > errno, strerror(errno)); > smp_wmb(); > @@ -227,7 +228,8 @@ int main(int argc, char *argv[]) > vm = vm_create_with_one_vcpu(&vcpu, guest_code); > ucall_init(vm, NULL); > > - pthread_create(&migration_thread, NULL, migration_worker, 0); > + pthread_create(&migration_thread, NULL, migration_worker, > + (void *)(unsigned long)gettid()); > > for (i = 0; !done; i++) { > vcpu_run(vcpu); > > base-commit: ad6cb756bb497997032df2bda7cbdff076e4a66a > -- >
| |