Re: [PATCH v1] KVM: selftests: Initalize sem_vcpu_[cont|stop] before each test in dirty_log_test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Oliver,

On 11/17/23 08:18, Oliver Upton wrote:
Hi Shaoqin,

On Thu, Nov 16, 2023 at 04:35:36AM -0500, Shaoqin Huang wrote:
When execute the dirty_log_test on some aarch64 machine, it sometimes
trigger the ASSERT:

==== Test Assertion Failure ====
   dirty_log_test.c:384: dirty_ring_vcpu_ring_full
   pid=14854 tid=14854 errno=22 - Invalid argument
      1  0x00000000004033eb: dirty_ring_collect_dirty_pages at dirty_log_test.c:384
      2  0x0000000000402d27: log_mode_collect_dirty_pages at dirty_log_test.c:505
      3   (inlined by) run_test at dirty_log_test.c:802
      4  0x0000000000403dc7: for_each_guest_mode at guest_modes.c:100
      5  0x0000000000401dff: main at dirty_log_test.c:941 (discriminator 3)
      6  0x0000ffff9be173c7: ?? ??:0
      7  0x0000ffff9be1749f: ?? ??:0
      8  0x000000000040206f: _start at ??:?
   Didn't continue vcpu even without ring full

The dirty_log_test fails when execute the dirty-ring test, this is
because the sem_vcpu_cont and the sem_vcpu_stop is non-zero value when
execute the dirty_ring_collect_dirty_pages() function. When those two
sem_t variables are non-zero, the dirty_ring_wait_vcpu() at the
beginning of the dirty_ring_collect_dirty_pages() will not wait for the
vcpu to stop, but continue to execute the following code. In this case,
before vcpu stop, if the dirty_ring_vcpu_ring_full is true, and the
dirty_ring_collect_dirty_pages() has passed the check for the
dirty_ring_vcpu_ring_full but hasn't execute the check for the
continued_vcpu, the vcpu stop, and set the dirty_ring_vcpu_ring_full to
false. Then dirty_ring_collect_dirty_pages() will trigger the ASSERT.

Why sem_vcpu_cont and sem_vcpu_stop can be non-zero value? It's because
the dirty_ring_before_vcpu_join() execute the sem_post(&sem_vcpu_cont)
at the end of each dirty-ring test. It can cause two cases:

1. sem_vcpu_cont be non-zero. When we set the host_quit to be true,
    the vcpu_worker directly see the host_quit to be true, it quit. So
    the log_mode_before_vcpu_join() function will set the sem_vcpu_cont
    to 1, since the vcpu_worker has quit, it won't consume it.
2. sem_vcpu_stop be non-zero. When we set the host_quit to be true,
    the vcpu_worker has entered the guest state, the next time it exit
    from guest state, it will set the sem_vcpu_stop to 1, and then see
    the host_quit, no one will consume the sem_vcpu_stop.

When execute more and more dirty-ring tests, the sem_vcpu_cont and
sem_vcpu_stop can be larger and larger, which makes many code paths
don't wait for the sem_t. Thus finally cause the problem.

Fix this problem is easy, simply initialize the sem_t before every test.
Thus whatever the state previous test left, it won't interfere the next
test.

In your changelog you describe what sounds like a semaphore imbalance at
the time of test completion, yet your proposed fix is to just clobber
the error and start fresh.


Yes. It's a semaphore imbalance problem.

Why not nip it at the bud and fix the logic bug instead?

I have another patch which fix the logic bug, I will send it out later.



--
Shaoqin





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux