Re: [RFC 1/1] KVM: selftests: rseq_test: use vdso_getcpu() instead of syscall()

Gavin Shan
2022-11-02T04:25:20Z

Hi Robert,

On 11/2/22 10:01 AM, Robert Hoo wrote:
> vDSO getcpu() has been in Kernel since 2.6.19, which we can assume
> generally available.
> Use vDSO getcpu() to reduce the overhead, so that vcpu thread stalls less
> therefore can have more odds to hit the race condition.
>

It would be nice to provide more context to explain how the race
condition is caused.

> Fixes: 0fcc102923de ("KVM: selftests: Use getcpu() instead of sched_getcpu() in rseq_test")
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> ---
> tools/testing/selftests/kvm/rseq_test.c | 32 ++++++++++++++++++-------
> 1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c
> index 6f88da7e60be..0b68a6b19b31 100644
> --- a/tools/testing/selftests/kvm/rseq_test.c
> +++ b/tools/testing/selftests/kvm/rseq_test.c
> @@ -42,15 +42,29 @@ static void guest_code(void)
> }
>
> /*
> - * We have to perform direct system call for getcpu() because it's
> - * not available until glic 2.29.
> + * getcpu() was added in kernel 2.6.19. glibc support wasn't there
> + * until glibc 2.29.
> + * We can direct call it from vdso to ease gblic dependency.
> + *
> + * vdso manipulation code refers from selftests/x86/test_vsyscall.c
> */
> -static void sys_getcpu(unsigned *cpu)
> -{
> - int r;
> +typedef long (*getcpu_t)(unsigned *, unsigned *, void *);
> +static getcpu_t vdso_getcpu;
>
> - r = syscall(__NR_getcpu, cpu, NULL, NULL);
> - TEST_ASSERT(!r, "getcpu failed, errno = %d (%s)", errno, strerror(errno));
> +static void init_vdso(void)
> +{
> + void *vdso = dlopen("linux-vdso.so.1", RTLD_LAZY | RTLD_LOCAL |
> + RTLD_NOLOAD);
> + if (!vdso)
> + vdso = dlopen("linux-gate.so.1", RTLD_LAZY | RTLD_LOCAL |
> + RTLD_NOLOAD);
> + if (!vdso)
> + TEST_ASSERT(!vdso, "failed to find vDSO\n");
> +
> + vdso_getcpu = (getcpu_t)dlsym(vdso, "__vdso_getcpu");
> + if (!vdso_getcpu)
> + TEST_ASSERT(!vdso_getcpu,
> + "failed to find __vdso_getcpu in vDSO\n");
> }
>

As the comments say, vdso manipulation code comes from selftests/x86/test_vsyscall.c.
I would guess 'linux-vdso.so.1' and 'linux-gate.so.1' are x86 specific. If I'm correct,
the test case will fail on other architectures, including ARM64.

> static int next_cpu(int cpu)
> @@ -205,6 +219,8 @@ int main(int argc, char *argv[])
> struct kvm_vcpu *vcpu;
> u32 cpu, rseq_cpu;
>
> + init_vdso();
> +
> /* Tell stdout not to buffer its content */
> setbuf(stdout, NULL);
>
> @@ -253,7 +269,7 @@ int main(int argc, char *argv[])
> * across the seq_cnt reads.
> */
> smp_rmb();
> - sys_getcpu(&cpu);
> + vdso_getcpu(&cpu, NULL, NULL);
> rseq_cpu = rseq_current_cpu_raw();
> smp_rmb();
> } while (snapshot != atomic_read(&seq_cnt));
>

Thanks,
Gavin