From: Weinan Liu <wnliu@stu.xmu.edu.cn>

Failed to assert '(dirty_gfns && ring_size)' in kvm_dirty_ring_reap_one if
the vcpu has not been finished to create yet. This bug occasionally occurs
when I open 200+ qemu instances on my 16G 6-cores x86 machine. And it must
be triggered if inserting a 'sleep(10)' into kvm_vcpu_thread_fn as below--

 static void *kvm_vcpu_thread_fn(void *arg)
 {
     CPUState *cpu = arg;
     int r;

     rcu_register_thread();

+    sleep(10);
     qemu_mutex_lock_iothread();
     qemu_thread_get_self(cpu->thread);
     cpu->thread_id = qemu_get_thread_id();
     cpu->can_do_io = 1;

where dirty ring reaper will wakeup but then a vcpu has not been finished
to create.

Signed-off-by: Weinan Liu <wnliu@stu.xmu.edu.cn>
---
 accel/kvm/kvm-all.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 7e6a6076b1..840da7630e 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -719,6 +719,15 @@ static uint64_t kvm_dirty_ring_reap_locked(KVMState *s, CPUState* cpu)
         total = kvm_dirty_ring_reap_one(s, cpu);
     } else {
         CPU_FOREACH(cpu) {
+            /*
+             * Must ensure kvm_init_vcpu is finished, so cpu->kvm_dirty_gfns is
+             * available.
+             */
+            while (cpu->created == false) {
+                qemu_mutex_unlock_iothread();
+                qemu_mutex_lock_iothread();
+            }
+
             total += kvm_dirty_ring_reap_one(s, cpu);
         }
     }
-- 
2.25.1

Sorry, this patch is wrong.
kvm_dirty_ring_reap_locked holds slots_lock, which may result in deadlock at the moment when modifying memory_region.

I am finding a better way to get known the finishing of all vcpus' creations before waking reaper up.


> -----原始邮件-----发件人:"Weinan Liu" <liu-weinan@qq.com>发送时间:2023-02-05 00:08:08 (星期日)收件人:qemu-devel@nongnu.org抄送:peterx@redhat.com, dgilbert@redhat.com, "Weinan Liu" <wnliu@stu.xmu.edu.cn>主题:[PATCH] KVM: dirty ring: check if vcpu is created before dirty_ring_reap_one
> 
> From: Weinan Liu <wnliu@stu.xmu.edu.cn>
> 
> Failed to assert '(dirty_gfns && ring_size)' in kvm_dirty_ring_reap_one if
> the vcpu has not been finished to create yet. This bug occasionally occurs
> when I open 200+ qemu instances on my 16G 6-cores x86 machine. And it must
> be triggered if inserting a 'sleep(10)' into kvm_vcpu_thread_fn as below--
> 
>  static void *kvm_vcpu_thread_fn(void *arg)
>  {
>      CPUState *cpu = arg;
>      int r;
> 
>      rcu_register_thread();
> 
> +    sleep(10);
>      qemu_mutex_lock_iothread();
>      qemu_thread_get_self(cpu->thread);
>      cpu->thread_id = qemu_get_thread_id();
>      cpu->can_do_io = 1;
> 
> where dirty ring reaper will wakeup but then a vcpu has not been finished
> to create.
> 
> Signed-off-by: Weinan Liu <wnliu@stu.xmu.edu.cn>
> ---
>  accel/kvm/kvm-all.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 7e6a6076b1..840da7630e 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -719,6 +719,15 @@ static uint64_t kvm_dirty_ring_reap_locked(KVMState *s, CPUState* cpu)
>          total = kvm_dirty_ring_reap_one(s, cpu);
>      } else {
>          CPU_FOREACH(cpu) {
> +            /*
> +             * Must ensure kvm_init_vcpu is finished, so cpu->kvm_dirty_gfns is
> +             * available.
> +             */
> +            while (cpu->created == false) {
> +                qemu_mutex_unlock_iothread();
> +                qemu_mutex_lock_iothread();
> +            }
> +
>              total += kvm_dirty_ring_reap_one(s, cpu);
>          }
>      }
> -- 
> 2.25.1


------------------------------
Weinan Liu （刘炜楠）

Department of Computer Science and Technology
School of Informatics Xiamen University

Sorry, this patch is wrong.
kvm_dirty_ring_reap_locked holds slots_lock, which may result in deadlock at the moment when modifying memory_region.

I am finding a better way to get known the finishing of all vcpus' creations before waking reaper up.


> -----原始邮件-----发件人:"Weinan Liu" <liu-weinan@qq.com>发送时间:2023-02-05 00:08:08 (星期日)收件人:qemu-devel@nongnu.org抄送:peterx@redhat.com, dgilbert@redhat.com, "Weinan Liu" <wnliu@stu.xmu.edu.cn>主题:[PATCH] KVM: dirty ring: check if vcpu is created before dirty_ring_reap_one
> 
> From: Weinan Liu <wnliu@stu.xmu.edu.cn>
> 
> Failed to assert '(dirty_gfns && ring_size)' in kvm_dirty_ring_reap_one if
> the vcpu has not been finished to create yet. This bug occasionally occurs
> when I open 200+ qemu instances on my 16G 6-cores x86 machine. And it must
> be triggered if inserting a 'sleep(10)' into kvm_vcpu_thread_fn as below--
> 
>  static void *kvm_vcpu_thread_fn(void *arg)
>  {
>      CPUState *cpu = arg;
>      int r;
> 
>      rcu_register_thread();
> 
> +    sleep(10);
>      qemu_mutex_lock_iothread();
>      qemu_thread_get_self(cpu->thread);
>      cpu->thread_id = qemu_get_thread_id();
>      cpu->can_do_io = 1;
> 
> where dirty ring reaper will wakeup but then a vcpu has not been finished
> to create.
> 
> Signed-off-by: Weinan Liu <wnliu@stu.xmu.edu.cn>
> ---
>  accel/kvm/kvm-all.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 7e6a6076b1..840da7630e 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -719,6 +719,15 @@ static uint64_t kvm_dirty_ring_reap_locked(KVMState *s, CPUState* cpu)
>          total = kvm_dirty_ring_reap_one(s, cpu);
>      } else {
>          CPU_FOREACH(cpu) {
> +            /*
> +             * Must ensure kvm_init_vcpu is finished, so cpu->kvm_dirty_gfns is
> +             * available.
> +             */
> +            while (cpu->created == false) {
> +                qemu_mutex_unlock_iothread();
> +                qemu_mutex_lock_iothread();
> +            }
> +
>              total += kvm_dirty_ring_reap_one(s, cpu);
>          }
>      }
> -- 
> 2.25.1


------------------------------
Weinan Liu （刘炜楠）

Department of Computer Science and Technology
School of Informatics Xiamen University