From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1132333D8 for ; Wed, 18 Dec 2024 00:01:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734480100; cv=none; b=QGb0c/3lS4zLwpIXkAfXC9StNoitdpQ4bW0owpcWyXszwHDGlmjpScNjjB4d9HAjlv/x8nU/d2elBAu1BeGlh1eNK5ZHdsun4lrsYnQWUxUBMdvxkD4xBdKvNLL4ugpeSbdeliTbhWtcoaOMvvFFvxbexvbEvXm5zwWsLFAYSus= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734480100; c=relaxed/simple; bh=vVMSarTL4N2s340QMRcYgMbsinxaLhfegPanrb6aBgw=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=KapLb7YO+sG6oEGV/IbnaZOYjEW54gsOMrtatjHooSCMtFBbZ2hvlIYJO7mdGAH7s4e2XVcJsjzmUWYxgnAUf3E71HapXAIqaV2jDpxVe1GvNDZcfrymsA1xBchuj9BPj7v0IY3cKcKj6ZaKWnVYDRDCElovDCzFxRCHAOjT8zA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=C55gIISP; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="C55gIISP" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734480098; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nvgZx2crNsqZt4BsRhiMLfgvWFUrdaXN/HwK/dKUNkw=; b=C55gIISPRWTQQ+xlqFKEtfBgZUcstgPKOS7Mas1dLoPMRKVjRX5IwPRoH6TyDH7wibVqEz TXDVeF80y/lc/HPxup49Srj9UYVqBVXsZhwu5iMKihtVR/a/SJpxi04uqmMhPgtZJeK/Of qx5mg5irdqCqXfNxhj1ADfohrF8Y2Q4= Received: from mail-io1-f69.google.com (mail-io1-f69.google.com [209.85.166.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-616-7tO7PO_4NySv3wwTd4EVLA-1; Tue, 17 Dec 2024 19:01:35 -0500 X-MC-Unique: 7tO7PO_4NySv3wwTd4EVLA-1 X-Mimecast-MFC-AGG-ID: 7tO7PO_4NySv3wwTd4EVLA Received: by mail-io1-f69.google.com with SMTP id ca18e2360f4ac-83e5dd390bfso21133239f.1 for ; Tue, 17 Dec 2024 16:01:35 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734480095; x=1735084895; h=content-transfer-encoding:mime-version:user-agent:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=nvgZx2crNsqZt4BsRhiMLfgvWFUrdaXN/HwK/dKUNkw=; b=t6tWmSH3oc9fJbplofSKxZPBgdSB2nRKw+IRqQspLvo8LbFP8maMu8Kwe0I5pjq+dt +p3dKMRPYOs5UZdf2C2YPt6HqkDDOIkn7R3s/nb1N9JtD2Me9/si4hxPbIHeuiQWmMlv UPl9RJyFYvriinhbZwc3TiRWGnsQj7f/58KICO+fcHJCerqU4vNIiw9jSi4U4f0Uf+FH i+w6Tr30MD68qDW54//wa0ZNYEXSQhCvb6w5i4BB7ep7REcQEkmrXCm595mOs6sAniAL aYnS1f2v8Y9snO64atq6bK8OmPAAYRTKDqMA2WCvsY/n5YF1lMySqlKki/8Y0kQK4kZ0 Odug== X-Forwarded-Encrypted: i=1; AJvYcCVblm4SL9hdar0pl2Iopz5oPi3NRULBJf20vc6HvLSy+NwxJukOq+sNcAhZ/IIocSCo5JJZyxwcoleg/KA=@vger.kernel.org X-Gm-Message-State: AOJu0YyyNyQxH44bMohSwxuKOPtvL1f0G2Z7hbXMmqCR6dofCJa5Q9vD lvYZQVC0/4BrLD4O5hMSPu2fjeNrFK0ku9aCcmWOkOm+l5H3NaprqcywjWhHvHtm5fMi58IROsO uVaf78o9ImnhWIaBtdNo4yzBqmjlbNfYPOcUr1FOsuJ8Jv4+N44WgcxlOcXm+RA== X-Gm-Gg: ASbGnctp0H6L2Racsq4lhvcsxnc/8aumwIwUGfC4SeuMA0va79IZKi4vgYy4QTIforK eOxDnewzvUnjdfBeHTxgZewng4kAY2tHYNNFn19egenaWY4wVmzFN9SVs0biWAPqJXvE0oyr7/s 7XDl+po/asPycKsza0E0ZOli/EYGkZtYPSezDya73khzGI8sagVU+FvVoTK0yBkE4P8dxLsLUzm /XyEFleUZohakRjtwYJMRkzeT42k7AzDBG9Yx0pdDiBy29KmpNq0i3F X-Received: by 2002:a05:6602:148e:b0:835:3dfc:5ba5 with SMTP id ca18e2360f4ac-84747e25760mr538484539f.5.1734480094815; Tue, 17 Dec 2024 16:01:34 -0800 (PST) X-Google-Smtp-Source: AGHT+IEgUSvLxLx8Jacs3zu4xrTuXPccVSmb7WefXa01BGBXeUW42nLC3tU6OvQezOhmtr+LiEBtEQ== X-Received: by 2002:a05:6602:148e:b0:835:3dfc:5ba5 with SMTP id ca18e2360f4ac-84747e25760mr538481939f.5.1734480094471; Tue, 17 Dec 2024 16:01:34 -0800 (PST) Received: from starship ([2607:fea8:fc01:8d8d:6adb:55ff:feaa:b156]) by smtp.gmail.com with ESMTPSA id ca18e2360f4ac-844f626a910sm199272439f.17.2024.12.17.16.01.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 17 Dec 2024 16:01:33 -0800 (PST) Message-ID: <336741c9f992bb340aa075f65578a0d4a68b0193.camel@redhat.com> Subject: Re: [PATCH 11/20] KVM: selftests: Post to sem_vcpu_stop if and only if vcpu_stop is true From: Maxim Levitsky To: Sean Christopherson , Paolo Bonzini Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Peter Xu Date: Tue, 17 Dec 2024 19:01:32 -0500 In-Reply-To: <20241214010721.2356923-12-seanjc@google.com> References: <20241214010721.2356923-1-seanjc@google.com> <20241214010721.2356923-12-seanjc@google.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5 (3.36.5-2.fc32) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 7bit On Fri, 2024-12-13 at 17:07 -0800, Sean Christopherson wrote: > When running dirty_log_test using the dirty ring, post to sem_vcpu_stop > only when the main thread has explicitly requested that the vCPU stop. > Synchronizing the vCPU and main thread whenever the dirty ring happens to > be full is unnecessary, as KVM's ABI is to actively prevent the vCPU from > running until the ring is no longer full. I.e. attempting to run the vCPU > will simply result in KVM_EXIT_DIRTY_RING_FULL without ever entering the > guest. And if KVM doesn't exit, e.g. let's the vCPU dirty more pages, > then that's a KVM bug worth finding. This is probably a good idea to do sometimes, but this can also reduce coverage because now the vCPU will pointlessly enter and exit when dirty log is full. Best regards, Maxim Levitsky > > Posting to sem_vcpu_stop on ring full also makes it difficult to get the > test logic right, e.g. it's easy to let the vCPU keep running when it > shouldn't, as a ring full can essentially happen at any given time. > > Opportunistically rework the handling of dirty_ring_vcpu_ring_full to > leave it set for the remainder of the iteration in order to simplify the > surrounding logic. > > Signed-off-by: Sean Christopherson > --- > tools/testing/selftests/kvm/dirty_log_test.c | 18 ++++-------------- > 1 file changed, 4 insertions(+), 14 deletions(-) > > diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c > index 40c8f5551c8e..8544e8425f9c 100644 > --- a/tools/testing/selftests/kvm/dirty_log_test.c > +++ b/tools/testing/selftests/kvm/dirty_log_test.c > @@ -379,12 +379,8 @@ static void dirty_ring_after_vcpu_run(struct kvm_vcpu *vcpu) > if (get_ucall(vcpu, NULL) == UCALL_SYNC) { > vcpu_handle_sync_stop(); > } else if (run->exit_reason == KVM_EXIT_DIRTY_RING_FULL) { > - /* Update the flag first before pause */ > WRITE_ONCE(dirty_ring_vcpu_ring_full, true); > - sem_post(&sem_vcpu_stop); > - pr_info("Dirty ring full, waiting for it to be collected\n"); > - sem_wait(&sem_vcpu_cont); > - WRITE_ONCE(dirty_ring_vcpu_ring_full, false); > + vcpu_handle_sync_stop(); > } else { > TEST_ASSERT(false, "Invalid guest sync status: " > "exit_reason=%s", > @@ -743,7 +739,6 @@ static void run_test(enum vm_guest_mode mode, void *arg) > pthread_create(&vcpu_thread, NULL, vcpu_worker, vcpu); > > while (iteration < p->iterations) { > - bool saw_dirty_ring_full = false; > unsigned long i; > > dirty_ring_prev_iteration_last_page = dirty_ring_last_page; > @@ -775,19 +770,12 @@ static void run_test(enum vm_guest_mode mode, void *arg) > * the ring on every pass would make it unlikely the > * vCPU would ever fill the fing). > */ > - if (READ_ONCE(dirty_ring_vcpu_ring_full)) > - saw_dirty_ring_full = true; > - if (i && !saw_dirty_ring_full) > + if (i && !READ_ONCE(dirty_ring_vcpu_ring_full)) > continue; > > log_mode_collect_dirty_pages(vcpu, TEST_MEM_SLOT_INDEX, > bmap, host_num_pages, > &ring_buf_idx); > - > - if (READ_ONCE(dirty_ring_vcpu_ring_full)) { > - pr_info("Dirty ring emptied, restarting vCPU\n"); > - sem_post(&sem_vcpu_cont); > - } > } > > /* > @@ -829,6 +817,8 @@ static void run_test(enum vm_guest_mode mode, void *arg) > WRITE_ONCE(host_quit, true); > sync_global_to_guest(vm, iteration); > > + WRITE_ONCE(dirty_ring_vcpu_ring_full, false); > + > sem_post(&sem_vcpu_cont); > } >