lkml.org 
[lkml]   [2022]   [Jul]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3] KVM: selftests: Fix target thread to be migrated in rseq_test
From
Date
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
> --
>

\
 
 \ /
  Last update: 2022-07-19 02:15    [W:0.085 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site