Re: [PATCH v3] KVM: selftests: Fix target thread to be migrated in rseq_test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx/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
--





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux