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

Gavin Shan
2022-11-03T01:18:06Z

On 11/3/22 8:46 AM, Sean Christopherson wrote:
> On Wed, Nov 02, 2022, 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.
>>
>> Fixes: 0fcc102923de ("KVM: selftests: Use getcpu() instead of sched_getcpu() in rseq_test")
>> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
>> ---
>
> ...
>
>> @@ -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));
>
> Something seems off here. Half of the iterations in the migration thread have a
> delay of 5+us, which should be more than enough time to complete a few getcpu()
> syscalls to stabilize the CPU.
>
> Has anyone tried to figure out why the vCPU thread is apparently running slow?
> E.g. is KVM_RUN itself taking a long time, is the task not getting scheduled in,
> etc... I can see how using vDSO would make the vCPU more efficient, but I'm
> curious as to why that's a problem in the first place.
>
> Anyways, assuming there's no underlying problem that can be solved, the easier
> solution is to just bump the delay in the migration thread. As per its gigantic
> comment, the original bug reproduced with up to 500us delays, so bumping the min
> delay to e.g. 5us is acceptable. If that doesn't guarantee the vCPU meets its
> quota, then something else is definitely going on.
>

I doubt if it's still caused by busy system as mentioned previously [1]. At least,
I failed to reproduce the issue on my ARM64 system until some workloads are enforced
to hog CPUs. Looking at the implementation syscall(NR_getcpu), it's simply to copy
the per-cpu data from kernel to userspace. So I don't see it should consume lots
of time. As system call is handled by interrupt/exception, the time consumed by
the interrupt/exception handler should be architecture dependent. Besides, the time
needed by ioctl(KVM_RUN) also differs on architectures.

[1] https://lore.kernel.org/kvm/d8290cbe-5d87-137a-0633-0ff5c69d57b0@redhat.com/

I think Sean's suggestion to bump the delay to 5us would be the quick fix if it helps.
However, more time will be needed to complete the test. Sean, do you mind to reduce
NR_TASK_MIGRATIONS from 100000 to 20000 either?

Thanks,
Gavin