Re: [PATCH] softmmu/dirtylimit: Fix usleep early return on signal

[Yong Huang <yong.huang@smartx.com>]

[-- Attachment #1: Type: text/plain, Size: 2553 bytes --]

On Fri, Sep 1, 2023 at 10:19 AM <alloc.young@outlook.com> wrote:

> From: alloc <alloc.young@outlook.com>
>
> Timeout functions like usleep can return early on signal, which reduces
> more dirty pages than expected. In dirtylimit case, dirtyrate meter
> thread needs to kick all vcpus out to sync. The callchain:
>
> vcpu_calculate_dirtyrate
>     global_dirty_log_sync
>         memory_global_dirty_log_sync
>             kvm_log_sync_global
>                 kvm_dirty_ring_flush
>                     kvm_cpu_synchronize_kick_all <---- send vcpu signal
>
> For long time sleep, use qemu_cond_timedwait_iothread to handle cpu stop
> event.
>
>
The Dirty Limit algorithm seeks to keep the vCPU dirty page rate within
the set limit; since it focuses more emphasis on processing time and
precision, I feel that improvement should strive for the same result.
Could you please provide the final test results showing the impact of
that improvement?


> Signed-off-by: alloc <alloc.young@outlook.com>
> ---
>  softmmu/dirtylimit.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
> index fa959d7743..ee938c636d 100644
> --- a/softmmu/dirtylimit.c
> +++ b/softmmu/dirtylimit.c
> @@ -411,13 +411,28 @@ void dirtylimit_set_all(uint64_t quota,
>
>  void dirtylimit_vcpu_execute(CPUState *cpu)
>  {
> +    int64_t sleep_us, endtime_us;
> +
> +    dirtylimit_state_lock();
>      if (dirtylimit_in_service() &&
>          dirtylimit_vcpu_get_state(cpu->cpu_index)->enabled &&
>          cpu->throttle_us_per_full) {
>          trace_dirtylimit_vcpu_execute(cpu->cpu_index,
>                  cpu->throttle_us_per_full);
> -        usleep(cpu->throttle_us_per_full);
> -    }
> +        sleep_us = cpu->throttle_us_per_full;
> +        dirtylimit_state_unlock();
> +        endtime_us = qemu_clock_get_us(QEMU_CLOCK_REALTIME) + sleep_us;
> +        while (sleep_us > 0 && !cpu->stop) {
> +            if (sleep_us > SCALE_US) {
> +                qemu_mutex_lock_iothread();
> +                qemu_cond_timedwait_iothread(cpu->halt_cond, sleep_us /
> SCALE_US);
> +                qemu_mutex_unlock_iothread();
> +            } else
> +                g_usleep(sleep_us);
> +            sleep_us = endtime_us -
> qemu_clock_get_us(QEMU_CLOCK_REALTIME);
> +        }
> +    } else
> +        dirtylimit_state_unlock();
>  }
>
>  static void dirtylimit_init(void)
> --
> 2.39.3
>
>

-- 
Best regards

[-- Attachment #2: Type: text/html, Size: 4366 bytes --]