kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] kvm/selftests: Two rseq_test fixes
@ 2022-08-10 10:41 Gavin Shan
  2022-08-10 10:41 ` [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35 Gavin Shan
  2022-08-10 10:41 ` [PATCH v2 2/2] KVM: selftests: Use getcpu() instead of sched_getcpu() in rseq_test Gavin Shan
  0 siblings, 2 replies; 7+ messages in thread
From: Gavin Shan @ 2022-08-10 10:41 UTC (permalink / raw)
  To: kvmarm
  Cc: kvm, linux-kernel, linux-kselftest, fweimer, shan.gavin, maz,
	andrew.jones, mathieu.desnoyers, pbonzini, yihyu, seanjc,
	oliver.upton

There are two issues in current rseq_test implementation and the
series intends to fix them:

- From glibc-2.35, rseq information is registered by TLS. It means
  rseq_test is unable to register its own rseq information. PATCH[01]
  fixes the issue by reusing "../rseq/rseq.c" to fetch TLS's rseq
  information if possible.

- sched_getcpu() relies on glibc's implementation and it can simply
  returns the CPU ID cached in the rseq information. In this case,
  it's pointless to compare the return value from sched_getcpu()
  and that fetched from rseq information. PATCH[02] fixes the issue
  by replacing sched_getcpu() with getcpu().

v1: https://lore.kernel.org/lkml/8c1f33b4-a5a1-fcfa-4521-36253ffa22c8@redhat.com/T/

Changelog
=========
v2:
  * Add "-ldl" to LDLIBS as Florian suggested.
  * Reuse "../rseq/rseq.c" as Paolo/Mathieu/Sean suggested.
  * Add comments to sys_getcpu() as Sean suggested.

Gavin Shan (2):
  KVM: selftests: Make rseq compatible with glibc-2.35
  KVM: selftests: Use getcpu() instead of sched_getcpu() in rseq_test

 tools/testing/selftests/kvm/Makefile    |  5 ++-
 tools/testing/selftests/kvm/rseq_test.c | 60 ++++++++++++-------------
 2 files changed, 33 insertions(+), 32 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35
  2022-08-10 10:41 [PATCH v2 0/2] kvm/selftests: Two rseq_test fixes Gavin Shan
@ 2022-08-10 10:41 ` Gavin Shan
  2022-08-10 12:22   ` Mathieu Desnoyers
  2022-08-10 10:41 ` [PATCH v2 2/2] KVM: selftests: Use getcpu() instead of sched_getcpu() in rseq_test Gavin Shan
  1 sibling, 1 reply; 7+ messages in thread
From: Gavin Shan @ 2022-08-10 10:41 UTC (permalink / raw)
  To: kvmarm
  Cc: kvm, linux-kernel, linux-kselftest, fweimer, shan.gavin, maz,
	andrew.jones, mathieu.desnoyers, pbonzini, yihyu, seanjc,
	oliver.upton

The rseq information is registered by TLS, starting from glibc-2.35.
In this case, the test always fails due to syscall(__NR_rseq). For
example, on RHEL9.1 where upstream glibc-2.35 features are enabled
on downstream glibc-2.34, the test fails like below.

  # ./rseq_test
  ==== Test Assertion Failure ====
    rseq_test.c:60: !r
    pid=112043 tid=112043 errno=22 - Invalid argument
       1        0x0000000000401973: main at rseq_test.c:226
       2        0x0000ffff84b6c79b: ?? ??:0
       3        0x0000ffff84b6c86b: ?? ??:0
       4        0x0000000000401b6f: _start at ??:?
    rseq failed, errno = 22 (Invalid argument)
  # rpm -aq | grep glibc-2
  glibc-2.34-39.el9.aarch64

Fix the issue by using "../rseq/rseq.c" to fetch the rseq information,
registred by TLS if it exists. Otherwise, we're going to register our
own rseq information as before.

Reported-by: Yihuang Yu <yihyu@redhat.com>
Suggested-by: Florian Weimer <fweimer@redhat.com>
Suggested-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 tools/testing/selftests/kvm/Makefile    |  5 +++--
 tools/testing/selftests/kvm/rseq_test.c | 28 +++++++------------------
 2 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index c7f47429d6cd..89c9a8c52c5f 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -197,7 +197,8 @@ endif
 CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
 	-fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \
 	-I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \
-	-I$(<D) -Iinclude/$(UNAME_M) -I.. $(EXTRA_CFLAGS) $(KHDR_INCLUDES)
+	-I$(<D) -Iinclude/$(UNAME_M) -I ../rseq -I.. $(EXTRA_CFLAGS) \
+	$(KHDR_INCLUDES)
 
 no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
         $(CC) -Werror -no-pie -x c - -o "$$TMP", -no-pie)
@@ -206,7 +207,7 @@ no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
 pgste-option = $(call try-run, echo 'int main() { return 0; }' | \
 	$(CC) -Werror -Wl$(comma)--s390-pgste -x c - -o "$$TMP",-Wl$(comma)--s390-pgste)
 
-
+LDLIBS += -ldl
 LDFLAGS += -pthread $(no-pie-option) $(pgste-option)
 
 # After inclusion, $(OUTPUT) is defined and
diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c
index a54d4d05a058..2cd5fe49ac8b 100644
--- a/tools/testing/selftests/kvm/rseq_test.c
+++ b/tools/testing/selftests/kvm/rseq_test.c
@@ -20,15 +20,7 @@
 #include "processor.h"
 #include "test_util.h"
 
-static __thread volatile struct rseq __rseq = {
-	.cpu_id = RSEQ_CPU_ID_UNINITIALIZED,
-};
-
-/*
- * Use an arbitrary, bogus signature for configuring rseq, this test does not
- * actually enter an rseq critical section.
- */
-#define RSEQ_SIG 0xdeadbeef
+#include "../rseq/rseq.c"
 
 /*
  * Any bug related to task migration is likely to be timing-dependent; perform
@@ -37,6 +29,7 @@ static __thread volatile struct rseq __rseq = {
 #define NR_TASK_MIGRATIONS 100000
 
 static pthread_t migration_thread;
+static struct rseq_abi *__rseq;
 static cpu_set_t possible_mask;
 static int min_cpu, max_cpu;
 static bool done;
@@ -49,14 +42,6 @@ static void guest_code(void)
 		GUEST_SYNC(0);
 }
 
-static void sys_rseq(int flags)
-{
-	int r;
-
-	r = syscall(__NR_rseq, &__rseq, sizeof(__rseq), flags, RSEQ_SIG);
-	TEST_ASSERT(!r, "rseq failed, errno = %d (%s)", errno, strerror(errno));
-}
-
 static int next_cpu(int cpu)
 {
 	/*
@@ -218,7 +203,10 @@ int main(int argc, char *argv[])
 
 	calc_min_max_cpu();
 
-	sys_rseq(0);
+	r = rseq_register_current_thread();
+	TEST_ASSERT(!r, "rseq_register_current_thread failed, errno = %d (%s)",
+		    errno, strerror(errno));
+	__rseq = rseq_get_abi();
 
 	/*
 	 * Create and run a dummy VM that immediately exits to userspace via
@@ -256,7 +244,7 @@ int main(int argc, char *argv[])
 			 */
 			smp_rmb();
 			cpu = sched_getcpu();
-			rseq_cpu = READ_ONCE(__rseq.cpu_id);
+			rseq_cpu = READ_ONCE(__rseq->cpu_id);
 			smp_rmb();
 		} while (snapshot != atomic_read(&seq_cnt));
 
@@ -278,7 +266,7 @@ int main(int argc, char *argv[])
 
 	kvm_vm_free(vm);
 
-	sys_rseq(RSEQ_FLAG_UNREGISTER);
+	rseq_unregister_current_thread();
 
 	return 0;
 }
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] KVM: selftests: Use getcpu() instead of sched_getcpu() in rseq_test
  2022-08-10 10:41 [PATCH v2 0/2] kvm/selftests: Two rseq_test fixes Gavin Shan
  2022-08-10 10:41 ` [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35 Gavin Shan
@ 2022-08-10 10:41 ` Gavin Shan
  1 sibling, 0 replies; 7+ messages in thread
From: Gavin Shan @ 2022-08-10 10:41 UTC (permalink / raw)
  To: kvmarm
  Cc: kvm, linux-kernel, linux-kselftest, fweimer, shan.gavin, maz,
	andrew.jones, mathieu.desnoyers, pbonzini, yihyu, seanjc,
	oliver.upton

sched_getcpu() is glibc dependent and it can simply return the CPU
ID from the registered rseq information, as Florian Weimer pointed.
In this case, it's pointless to compare the return value from
sched_getcpu() and that fetched from the registered rseq information.

Fix the issue by replacing sched_getcpu() with getcpu(), as Florian
suggested. The comments are modified accordingly by replacing
"sched_getcpu()" with "getcpu()".

Reported-by: Yihuang Yu <yihyu@redhat.com>
Suggested-by: Florian Weimer <fweimer@redhat.com>
Suggested-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 tools/testing/selftests/kvm/rseq_test.c | 42 ++++++++++++++++---------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c
index 2cd5fe49ac8b..84e8425edc2c 100644
--- a/tools/testing/selftests/kvm/rseq_test.c
+++ b/tools/testing/selftests/kvm/rseq_test.c
@@ -42,6 +42,18 @@ static void guest_code(void)
 		GUEST_SYNC(0);
 }
 
+/*
+ * We have to perform direct system call for getcpu() because it's
+ * not available until glic 2.29.
+ */
+static void sys_getcpu(unsigned *cpu)
+{
+	int r;
+
+	r = syscall(__NR_getcpu, cpu, NULL, NULL);
+	TEST_ASSERT(!r, "getcpu failed, errno = %d (%s)", errno, strerror(errno));
+}
+
 static int next_cpu(int cpu)
 {
 	/*
@@ -86,7 +98,7 @@ static void *migration_worker(void *__rseq_tid)
 		atomic_inc(&seq_cnt);
 
 		/*
-		 * Ensure the odd count is visible while sched_getcpu() isn't
+		 * Ensure the odd count is visible while getcpu() isn't
 		 * stable, i.e. while changing affinity is in-progress.
 		 */
 		smp_wmb();
@@ -127,10 +139,10 @@ static void *migration_worker(void *__rseq_tid)
 		 *     check completes.
 		 *
 		 *  3. To ensure the read-side makes efficient forward progress,
-		 *     e.g. if sched_getcpu() involves a syscall.  Stalling the
-		 *     read-side means the test will spend more time waiting for
-		 *     sched_getcpu() to stabilize and less time trying to hit
-		 *     the timing-dependent bug.
+		 *     e.g. if getcpu() involves a syscall. Stalling the read-side
+		 *     means the test will spend more time waiting for getcpu()
+		 *     to stabilize and less time trying to hit the timing-dependent
+		 *     bug.
 		 *
 		 * Because any bug in this area is likely to be timing-dependent,
 		 * run with a range of delays at 1us intervals from 1us to 10us
@@ -226,9 +238,9 @@ int main(int argc, char *argv[])
 
 		/*
 		 * Verify rseq's CPU matches sched's CPU.  Ensure migration
-		 * doesn't occur between sched_getcpu() and reading the rseq
-		 * cpu_id by rereading both if the sequence count changes, or
-		 * if the count is odd (migration in-progress).
+		 * doesn't occur between getcpu() and reading the rseq cpu_id
+		 * by rereading both if the sequence count changes, or if the
+		 * count is odd (migration in-progress).
 		 */
 		do {
 			/*
@@ -238,12 +250,12 @@ int main(int argc, char *argv[])
 			snapshot = atomic_read(&seq_cnt) & ~1;
 
 			/*
-			 * Ensure reading sched_getcpu() and rseq.cpu_id
-			 * complete in a single "no migration" window, i.e. are
-			 * not reordered across the seq_cnt reads.
+			 * Ensure reading getcpu() and rseq.cpu_id complete in
+			 * a single "no migration" window, i.e. are not reordered
+			 * across the seq_cnt reads.
 			 */
 			smp_rmb();
-			cpu = sched_getcpu();
+			sys_getcpu(&cpu);
 			rseq_cpu = READ_ONCE(__rseq->cpu_id);
 			smp_rmb();
 		} while (snapshot != atomic_read(&seq_cnt));
@@ -255,9 +267,9 @@ int main(int argc, char *argv[])
 	/*
 	 * Sanity check that the test was able to enter the guest a reasonable
 	 * number of times, e.g. didn't get stalled too often/long waiting for
-	 * sched_getcpu() to stabilize.  A 2:1 migration:KVM_RUN ratio is a
-	 * fairly conservative ratio on x86-64, which can do _more_ KVM_RUNs
-	 * than migrations given the 1us+ delay in the migration task.
+	 * getcpu() to stabilize.  A 2:1 migration:KVM_RUN ratio is a fairly
+	 * conservative ratio on x86-64, which can do _more_ KVM_RUNs than
+	 * migrations given the 1us+ delay in the migration task.
 	 */
 	TEST_ASSERT(i > (NR_TASK_MIGRATIONS / 2),
 		    "Only performed %d KVM_RUNs, task stalled too much?\n", i);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35
  2022-08-10 10:41 ` [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35 Gavin Shan
@ 2022-08-10 12:22   ` Mathieu Desnoyers
  2022-08-10 12:29     ` Paolo Bonzini
  0 siblings, 1 reply; 7+ messages in thread
From: Mathieu Desnoyers @ 2022-08-10 12:22 UTC (permalink / raw)
  To: Gavin Shan
  Cc: kvmarm, KVM list, linux-kernel, linux-kselftest, Florian Weimer,
	shan gavin, maz, andrew jones, Paolo Bonzini, yihyu,
	Sean Christopherson, oliver upton

----- On Aug 10, 2022, at 6:41 AM, Gavin Shan gshan@redhat.com wrote:

> The rseq information is registered by TLS, starting from glibc-2.35.
> In this case, the test always fails due to syscall(__NR_rseq). For
> example, on RHEL9.1 where upstream glibc-2.35 features are enabled
> on downstream glibc-2.34, the test fails like below.
> 
>  # ./rseq_test
>  ==== Test Assertion Failure ====
>    rseq_test.c:60: !r
>    pid=112043 tid=112043 errno=22 - Invalid argument
>       1        0x0000000000401973: main at rseq_test.c:226
>       2        0x0000ffff84b6c79b: ?? ??:0
>       3        0x0000ffff84b6c86b: ?? ??:0
>       4        0x0000000000401b6f: _start at ??:?
>    rseq failed, errno = 22 (Invalid argument)
>  # rpm -aq | grep glibc-2
>  glibc-2.34-39.el9.aarch64
> 
> Fix the issue by using "../rseq/rseq.c" to fetch the rseq information,
> registred by TLS if it exists. Otherwise, we're going to register our
> own rseq information as before.
> 
> Reported-by: Yihuang Yu <yihyu@redhat.com>
> Suggested-by: Florian Weimer <fweimer@redhat.com>
> Suggested-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> tools/testing/selftests/kvm/Makefile    |  5 +++--
> tools/testing/selftests/kvm/rseq_test.c | 28 +++++++------------------
> 2 files changed, 11 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/Makefile
> b/tools/testing/selftests/kvm/Makefile
> index c7f47429d6cd..89c9a8c52c5f 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -197,7 +197,8 @@ endif
> CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
> 	-fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \
> 	-I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \
> -	-I$(<D) -Iinclude/$(UNAME_M) -I.. $(EXTRA_CFLAGS) $(KHDR_INCLUDES)
> +	-I$(<D) -Iinclude/$(UNAME_M) -I ../rseq -I.. $(EXTRA_CFLAGS) \
> +	$(KHDR_INCLUDES)
> 
> no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
>         $(CC) -Werror -no-pie -x c - -o "$$TMP", -no-pie)
> @@ -206,7 +207,7 @@ no-pie-option := $(call try-run, echo 'int main() { return
> 0; }' | \
> pgste-option = $(call try-run, echo 'int main() { return 0; }' | \
> 	$(CC) -Werror -Wl$(comma)--s390-pgste -x c - -o "$$TMP",-Wl$(comma)--s390-pgste)
> 
> -
> +LDLIBS += -ldl
> LDFLAGS += -pthread $(no-pie-option) $(pgste-option)
> 
> # After inclusion, $(OUTPUT) is defined and
> diff --git a/tools/testing/selftests/kvm/rseq_test.c
> b/tools/testing/selftests/kvm/rseq_test.c
> index a54d4d05a058..2cd5fe49ac8b 100644
> --- a/tools/testing/selftests/kvm/rseq_test.c
> +++ b/tools/testing/selftests/kvm/rseq_test.c
> @@ -20,15 +20,7 @@
> #include "processor.h"
> #include "test_util.h"
> 
> -static __thread volatile struct rseq __rseq = {
> -	.cpu_id = RSEQ_CPU_ID_UNINITIALIZED,
> -};
> -
> -/*
> - * Use an arbitrary, bogus signature for configuring rseq, this test does not
> - * actually enter an rseq critical section.
> - */
> -#define RSEQ_SIG 0xdeadbeef
> +#include "../rseq/rseq.c"
> 
> /*
>  * Any bug related to task migration is likely to be timing-dependent; perform
> @@ -37,6 +29,7 @@ static __thread volatile struct rseq __rseq = {
> #define NR_TASK_MIGRATIONS 100000
> 
> static pthread_t migration_thread;
> +static struct rseq_abi *__rseq;

What is this ?

> static cpu_set_t possible_mask;
> static int min_cpu, max_cpu;
> static bool done;
> @@ -49,14 +42,6 @@ static void guest_code(void)
> 		GUEST_SYNC(0);
> }
> 
> -static void sys_rseq(int flags)
> -{
> -	int r;
> -
> -	r = syscall(__NR_rseq, &__rseq, sizeof(__rseq), flags, RSEQ_SIG);
> -	TEST_ASSERT(!r, "rseq failed, errno = %d (%s)", errno, strerror(errno));
> -}
> -
> static int next_cpu(int cpu)
> {
> 	/*
> @@ -218,7 +203,10 @@ int main(int argc, char *argv[])
> 
> 	calc_min_max_cpu();
> 
> -	sys_rseq(0);
> +	r = rseq_register_current_thread();
> +	TEST_ASSERT(!r, "rseq_register_current_thread failed, errno = %d (%s)",
> +		    errno, strerror(errno));
> +	__rseq = rseq_get_abi();
> 
> 	/*
> 	 * Create and run a dummy VM that immediately exits to userspace via
> @@ -256,7 +244,7 @@ int main(int argc, char *argv[])
> 			 */
> 			smp_rmb();
> 			cpu = sched_getcpu();
> -			rseq_cpu = READ_ONCE(__rseq.cpu_id);
> +			rseq_cpu = READ_ONCE(__rseq->cpu_id);

#include <rseq.h>

and use

rseq_current_cpu_raw().

Thanks,

Mathieu


> 			smp_rmb();
> 		} while (snapshot != atomic_read(&seq_cnt));
> 
> @@ -278,7 +266,7 @@ int main(int argc, char *argv[])
> 
> 	kvm_vm_free(vm);
> 
> -	sys_rseq(RSEQ_FLAG_UNREGISTER);
> +	rseq_unregister_current_thread();
> 
> 	return 0;
> }
> --
> 2.23.0

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35
  2022-08-10 12:22   ` Mathieu Desnoyers
@ 2022-08-10 12:29     ` Paolo Bonzini
  2022-08-10 12:30       ` Mathieu Desnoyers
  2022-08-10 23:57       ` Gavin Shan
  0 siblings, 2 replies; 7+ messages in thread
From: Paolo Bonzini @ 2022-08-10 12:29 UTC (permalink / raw)
  To: Mathieu Desnoyers, Gavin Shan
  Cc: kvmarm, KVM list, linux-kernel, linux-kselftest, Florian Weimer,
	shan gavin, maz, andrew jones, yihyu, Sean Christopherson,
	oliver upton

On 8/10/22 14:22, Mathieu Desnoyers wrote:
>>
>> 	/*
>> 	 * Create and run a dummy VM that immediately exits to userspace via
>> @@ -256,7 +244,7 @@ int main(int argc, char *argv[])
>> 			 */
>> 			smp_rmb();
>> 			cpu = sched_getcpu();
>> -			rseq_cpu = READ_ONCE(__rseq.cpu_id);
>> +			rseq_cpu = READ_ONCE(__rseq->cpu_id);
> #include <rseq.h>
> 
> and use
> 
> rseq_current_cpu_raw().

Thanks, I squashed it and queued it for -rc1 (tested on both
glibc 2.34 and 2.35).

diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c
index 84e8425edc2c..987a76674f4f 100644
--- a/tools/testing/selftests/kvm/rseq_test.c
+++ b/tools/testing/selftests/kvm/rseq_test.c
@@ -29,7 +29,6 @@
  #define NR_TASK_MIGRATIONS 100000
  
  static pthread_t migration_thread;
-static struct rseq_abi *__rseq;
  static cpu_set_t possible_mask;
  static int min_cpu, max_cpu;
  static bool done;
@@ -218,7 +217,6 @@ int main(int argc, char *argv[])
  	r = rseq_register_current_thread();
  	TEST_ASSERT(!r, "rseq_register_current_thread failed, errno = %d (%s)",
  		    errno, strerror(errno));
-	__rseq = rseq_get_abi();
  
  	/*
  	 * Create and run a dummy VM that immediately exits to userspace via
@@ -256,7 +254,7 @@ int main(int argc, char *argv[])
  			 */
  			smp_rmb();
  			cpu = sched_getcpu();
-			rseq_cpu = READ_ONCE(__rseq->cpu_id);
+			rseq_cpu = rseq_current_cpu_raw();
  			smp_rmb();
  		} while (snapshot != atomic_read(&seq_cnt));
  

Paolo


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35
  2022-08-10 12:29     ` Paolo Bonzini
@ 2022-08-10 12:30       ` Mathieu Desnoyers
  2022-08-10 23:57       ` Gavin Shan
  1 sibling, 0 replies; 7+ messages in thread
From: Mathieu Desnoyers @ 2022-08-10 12:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Gavin Shan, kvmarm, KVM list, linux-kernel, linux-kselftest,
	Florian Weimer, shan gavin, maz, andrew jones, yihyu,
	Sean Christopherson, oliver upton

----- On Aug 10, 2022, at 8:29 AM, Paolo Bonzini pbonzini@redhat.com wrote:

> On 8/10/22 14:22, Mathieu Desnoyers wrote:
>>>
>>> 	/*
>>> 	 * Create and run a dummy VM that immediately exits to userspace via
>>> @@ -256,7 +244,7 @@ int main(int argc, char *argv[])
>>> 			 */
>>> 			smp_rmb();
>>> 			cpu = sched_getcpu();
>>> -			rseq_cpu = READ_ONCE(__rseq.cpu_id);
>>> +			rseq_cpu = READ_ONCE(__rseq->cpu_id);
>> #include <rseq.h>
>> 
>> and use
>> 
>> rseq_current_cpu_raw().
> 
> Thanks, I squashed it and queued it for -rc1 (tested on both
> glibc 2.34 and 2.35).

Thanks a lot Paolo,

Cheers! :)

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35
  2022-08-10 12:29     ` Paolo Bonzini
  2022-08-10 12:30       ` Mathieu Desnoyers
@ 2022-08-10 23:57       ` Gavin Shan
  1 sibling, 0 replies; 7+ messages in thread
From: Gavin Shan @ 2022-08-10 23:57 UTC (permalink / raw)
  To: Paolo Bonzini, Mathieu Desnoyers
  Cc: kvmarm, KVM list, linux-kernel, linux-kselftest, Florian Weimer,
	shan gavin, maz, andrew jones, yihyu, Sean Christopherson,
	oliver upton

On 8/10/22 10:29 PM, Paolo Bonzini wrote:
> On 8/10/22 14:22, Mathieu Desnoyers wrote:
>>>
>>>     /*
>>>      * Create and run a dummy VM that immediately exits to userspace via
>>> @@ -256,7 +244,7 @@ int main(int argc, char *argv[])
>>>              */
>>>             smp_rmb();
>>>             cpu = sched_getcpu();
>>> -            rseq_cpu = READ_ONCE(__rseq.cpu_id);
>>> +            rseq_cpu = READ_ONCE(__rseq->cpu_id);
>> #include <rseq.h>
>>
>> and use
>>
>> rseq_current_cpu_raw().
> 
> Thanks, I squashed it and queued it for -rc1 (tested on both
> glibc 2.34 and 2.35).
> 

Paolo, Thanks for the makeup, which looks good to me :)

> diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c
> index 84e8425edc2c..987a76674f4f 100644
> --- a/tools/testing/selftests/kvm/rseq_test.c
> +++ b/tools/testing/selftests/kvm/rseq_test.c
> @@ -29,7 +29,6 @@
>   #define NR_TASK_MIGRATIONS 100000
> 
>   static pthread_t migration_thread;
> -static struct rseq_abi *__rseq;
>   static cpu_set_t possible_mask;
>   static int min_cpu, max_cpu;
>   static bool done;
> @@ -218,7 +217,6 @@ int main(int argc, char *argv[])
>       r = rseq_register_current_thread();
>       TEST_ASSERT(!r, "rseq_register_current_thread failed, errno = %d (%s)",
>               errno, strerror(errno));
> -    __rseq = rseq_get_abi();
> 
>       /*
>        * Create and run a dummy VM that immediately exits to userspace via
> @@ -256,7 +254,7 @@ int main(int argc, char *argv[])
>                */
>               smp_rmb();
>               cpu = sched_getcpu();
> -            rseq_cpu = READ_ONCE(__rseq->cpu_id);
> +            rseq_cpu = rseq_current_cpu_raw();
>               smp_rmb();
>           } while (snapshot != atomic_read(&seq_cnt));
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-08-10 23:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-10 10:41 [PATCH v2 0/2] kvm/selftests: Two rseq_test fixes Gavin Shan
2022-08-10 10:41 ` [PATCH v2 1/2] KVM: selftests: Make rseq compatible with glibc-2.35 Gavin Shan
2022-08-10 12:22   ` Mathieu Desnoyers
2022-08-10 12:29     ` Paolo Bonzini
2022-08-10 12:30       ` Mathieu Desnoyers
2022-08-10 23:57       ` Gavin Shan
2022-08-10 10:41 ` [PATCH v2 2/2] KVM: selftests: Use getcpu() instead of sched_getcpu() in rseq_test Gavin Shan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).