From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

Add a non-required argument 'CPUState' to kvm_dirty_ring_reap so
that it can cover single vcpu dirty ring reaping scenario.

Only just reaping the target vcpu dirty ring instead of all when
vcpu thread return to user space caused by KVM_EXIT_DIRTY_RING_FULL.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 accel/kvm/kvm-all.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 0e66ebb..1a5f1d1 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -756,17 +756,20 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu)
 }
 
 /* Must be with slots_lock held */
-static uint64_t kvm_dirty_ring_reap_locked(KVMState *s)
+static uint64_t kvm_dirty_ring_reap_locked(KVMState *s, CPUState* cpu)
 {
     int ret;
-    CPUState *cpu;
     uint64_t total = 0;
     int64_t stamp;
 
     stamp = get_clock();
 
-    CPU_FOREACH(cpu) {
-        total += kvm_dirty_ring_reap_one(s, cpu);
+    if (cpu) {
+        total = kvm_dirty_ring_reap_one(s, cpu);
+    } else {
+        CPU_FOREACH(cpu) {
+            total += kvm_dirty_ring_reap_one(s, cpu);
+        }
     }
 
     if (total) {
@@ -787,7 +790,7 @@ static uint64_t kvm_dirty_ring_reap_locked(KVMState *s)
  * Currently for simplicity, we must hold BQL before calling this.  We can
  * consider to drop the BQL if we're clear with all the race conditions.
  */
-static uint64_t kvm_dirty_ring_reap(KVMState *s)
+static uint64_t kvm_dirty_ring_reap(KVMState *s, CPUState *cpu)
 {
     uint64_t total;
 
@@ -807,7 +810,7 @@ static uint64_t kvm_dirty_ring_reap(KVMState *s)
      *     reset below.
      */
     kvm_slots_lock();
-    total = kvm_dirty_ring_reap_locked(s);
+    total = kvm_dirty_ring_reap_locked(s, cpu);
     kvm_slots_unlock();
 
     return total;
@@ -854,7 +857,7 @@ static void kvm_dirty_ring_flush(void)
      * vcpus out in a synchronous way.
      */
     kvm_cpu_synchronize_kick_all();
-    kvm_dirty_ring_reap(kvm_state);
+    kvm_dirty_ring_reap(kvm_state, NULL);
     trace_kvm_dirty_ring_flush(1);
 }
 
@@ -1398,7 +1401,7 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
                  * Not easy.  Let's cross the fingers until it's fixed.
                  */
                 if (kvm_state->kvm_dirty_ring_size) {
-                    kvm_dirty_ring_reap_locked(kvm_state);
+                    kvm_dirty_ring_reap_locked(kvm_state, NULL);
                 } else {
                     kvm_slot_get_dirty_log(kvm_state, mem);
                 }
@@ -1470,7 +1473,7 @@ static void *kvm_dirty_ring_reaper_thread(void *data)
         r->reaper_state = KVM_DIRTY_RING_REAPER_REAPING;
 
         qemu_mutex_lock_iothread();
-        kvm_dirty_ring_reap(s);
+        kvm_dirty_ring_reap(s, NULL);
         qemu_mutex_unlock_iothread();
 
         r->reaper_iteration++;
@@ -2956,7 +2959,7 @@ int kvm_cpu_exec(CPUState *cpu)
              */
             trace_kvm_dirty_ring_full(cpu->cpu_index);
             qemu_mutex_lock_iothread();
-            kvm_dirty_ring_reap(kvm_state);
+            kvm_dirty_ring_reap(kvm_state, cpu);
             qemu_mutex_unlock_iothread();
             ret = 0;
             break;
-- 
1.8.3.1

On Mon, Jan 24, 2022 at 10:10:36PM +0800, huangy81@chinatelecom.cn wrote:
> @@ -2956,7 +2959,7 @@ int kvm_cpu_exec(CPUState *cpu)
>               */
>              trace_kvm_dirty_ring_full(cpu->cpu_index);
>              qemu_mutex_lock_iothread();
> -            kvm_dirty_ring_reap(kvm_state);
> +            kvm_dirty_ring_reap(kvm_state, cpu);

Shall we keep passing in NULL in this patch, and make it conditionally taking
cpu parameter if dirty limit enabled?

Ring reset can still be expensive, so ideally we can still try the best to reap
as much PFNs as possible, as long as we still don't need accuracy on RING_FULL
exit events.

>              qemu_mutex_unlock_iothread();
>              ret = 0;
>              break;
> -- 
> 1.8.3.1
> 

-- 
Peter Xu

在 2022/2/8 15:20, Peter Xu 写道:
> On Mon, Jan 24, 2022 at 10:10:36PM +0800, huangy81@chinatelecom.cn wrote:
>> @@ -2956,7 +2959,7 @@ int kvm_cpu_exec(CPUState *cpu)
>>                */
>>               trace_kvm_dirty_ring_full(cpu->cpu_index);
>>               qemu_mutex_lock_iothread();
>> -            kvm_dirty_ring_reap(kvm_state);
>> +            kvm_dirty_ring_reap(kvm_state, cpu);
> 
> Shall we keep passing in NULL in this patch, and make it conditionally taking
> cpu parameter if dirty limit enabled?
> 
Ok，so we should pass the cpu parameter only if dirtylimit in service.
> Ring reset can still be expensive, so ideally we can still try the best to reap
> as much PFNs as possible, as long as we still don't need accuracy on RING_FULL
> exit events.
> 
>>               qemu_mutex_unlock_iothread();
>>               ret = 0;
>>               break;
>> -- 
>> 1.8.3.1
>>
> 

-- 
Best regard

Hyman Huang(黄勇)