lkml.org 
[lkml]   [2021]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] KVM: selftests: Ensure all migrations are performed when test is affined
From
On 30/09/21 01:41, Sean Christopherson wrote:
> Rework the CPU selection in the migration worker to ensure the specified
> number of migrations are performed when the test iteslf is affined to a
> subset of CPUs. The existing logic skips iterations if the target CPU is
> not in the original set of possible CPUs, which causes the test to fail
> if too many iterations are skipped.
>
> ==== Test Assertion Failure ====
> rseq_test.c:228: i > (NR_TASK_MIGRATIONS / 2)
> pid=10127 tid=10127 errno=4 - Interrupted system call
> 1 0x00000000004018e5: main at rseq_test.c:227
> 2 0x00007fcc8fc66bf6: ?? ??:0
> 3 0x0000000000401959: _start at ??:?
> Only performed 4 KVM_RUNs, task stalled too much?
>
> Calculate the min/max possible CPUs as a cheap "best effort" to avoid
> high runtimes when the test is affined to a small percentage of CPUs.
> Alternatively, a list or xarray of the possible CPUs could be used, but
> even in a horrendously inefficient setup, such optimizations are not
> needed because the runtime is completely dominated by the cost of
> migrating the task, and the absolute runtime is well under a minute in
> even truly absurd setups, e.g. running on a subset of vCPUs in a VM that
> is heavily overcommited (16 vCPUs per pCPU).
>
> Fixes: 61e52f1630f5 ("KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs")
> Reported-by: Dongli Zhang <dongli.zhang@oracle.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> tools/testing/selftests/kvm/rseq_test.c | 69 +++++++++++++++++++++----
> 1 file changed, 59 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c
> index c5e0dd664a7b..4158da0da2bb 100644
> --- a/tools/testing/selftests/kvm/rseq_test.c
> +++ b/tools/testing/selftests/kvm/rseq_test.c
> @@ -10,6 +10,7 @@
> #include <signal.h>
> #include <syscall.h>
> #include <sys/ioctl.h>
> +#include <sys/sysinfo.h>
> #include <asm/barrier.h>
> #include <linux/atomic.h>
> #include <linux/rseq.h>
> @@ -39,6 +40,7 @@ static __thread volatile struct rseq __rseq = {
>
> static pthread_t migration_thread;
> static cpu_set_t possible_mask;
> +static int min_cpu, max_cpu;
> static bool done;
>
> static atomic_t seq_cnt;
> @@ -57,20 +59,37 @@ static void sys_rseq(int flags)
> TEST_ASSERT(!r, "rseq failed, errno = %d (%s)", errno, strerror(errno));
> }
>
> +static int next_cpu(int cpu)
> +{
> + /*
> + * Advance to the next CPU, skipping those that weren't in the original
> + * affinity set. Sadly, there is no CPU_SET_FOR_EACH, and cpu_set_t's
> + * data storage is considered as opaque. Note, if this task is pinned
> + * to a small set of discontigous CPUs, e.g. 2 and 1023, this loop will
> + * burn a lot cycles and the test will take longer than normal to
> + * complete.
> + */
> + do {
> + cpu++;
> + if (cpu > max_cpu) {
> + cpu = min_cpu;
> + TEST_ASSERT(CPU_ISSET(cpu, &possible_mask),
> + "Min CPU = %d must always be usable", cpu);
> + break;
> + }
> + } while (!CPU_ISSET(cpu, &possible_mask));
> +
> + return cpu;
> +}
> +
> static void *migration_worker(void *ign)
> {
> cpu_set_t allowed_mask;
> - int r, i, nr_cpus, cpu;
> + int r, i, cpu;
>
> CPU_ZERO(&allowed_mask);
>
> - nr_cpus = CPU_COUNT(&possible_mask);
> -
> - for (i = 0; i < NR_TASK_MIGRATIONS; i++) {
> - cpu = i % nr_cpus;
> - if (!CPU_ISSET(cpu, &possible_mask))
> - continue;
> -
> + for (i = 0, cpu = min_cpu; i < NR_TASK_MIGRATIONS; i++, cpu = next_cpu(cpu)) {
> CPU_SET(cpu, &allowed_mask);
>
> /*
> @@ -154,6 +173,36 @@ static void *migration_worker(void *ign)
> return NULL;
> }
>
> +static int calc_min_max_cpu(void)
> +{
> + int i, cnt, nproc;
> +
> + if (CPU_COUNT(&possible_mask) < 2)
> + return -EINVAL;
> +
> + /*
> + * CPU_SET doesn't provide a FOR_EACH helper, get the min/max CPU that
> + * this task is affined to in order to reduce the time spent querying
> + * unusable CPUs, e.g. if this task is pinned to a small percentage of
> + * total CPUs.
> + */
> + nproc = get_nprocs_conf();
> + min_cpu = -1;
> + max_cpu = -1;
> + cnt = 0;
> +
> + for (i = 0; i < nproc; i++) {
> + if (!CPU_ISSET(i, &possible_mask))
> + continue;
> + if (min_cpu == -1)
> + min_cpu = i;
> + max_cpu = i;
> + cnt++;
> + }
> +
> + return (cnt < 2) ? -EINVAL : 0;
> +}
> +
> int main(int argc, char *argv[])
> {
> int r, i, snapshot;
> @@ -167,8 +216,8 @@ int main(int argc, char *argv[])
> TEST_ASSERT(!r, "sched_getaffinity failed, errno = %d (%s)", errno,
> strerror(errno));
>
> - if (CPU_COUNT(&possible_mask) < 2) {
> - print_skip("Only one CPU, task migration not possible\n");
> + if (calc_min_max_cpu()) {
> + print_skip("Only one usable CPU, task migration not possible");
> exit(KSFT_SKIP);
> }
>
>

Queued, thanks.

Paolo

\
 
 \ /
  Last update: 2021-09-30 10:27    [W:3.080 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site