Re: [PATCH v1 4/8] migration: Implement dirty-limit convergence algo

[Hyman <huangy81@chinatelecom.cn>]

在 2022/9/7 4:37, Peter Xu 写道:
> On Fri, Sep 02, 2022 at 01:22:32AM +0800, huangy81@chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> Implement dirty-limit convergence algo for live migration,
>> which is kind of like auto-converge algo but using dirty-limit
>> instead of cpu throttle to make migration convergent.
>>
>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>> ---
>>   migration/migration.c  |  1 +
>>   migration/ram.c        | 53 +++++++++++++++++++++++++++++++++++++-------------
>>   migration/trace-events |  1 +
>>   3 files changed, 42 insertions(+), 13 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index d117bb4..64696de 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -239,6 +239,7 @@ void migration_cancel(const Error *error)
>>       if (error) {
>>           migrate_set_error(current_migration, error);
>>       }
>> +    qmp_cancel_vcpu_dirty_limit(false, -1, NULL);
>>       migrate_fd_cancel(current_migration);
>>   }
>>   
>> diff --git a/migration/ram.c b/migration/ram.c
>> index dc1de9d..cc19c5e 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -45,6 +45,7 @@
>>   #include "qapi/error.h"
>>   #include "qapi/qapi-types-migration.h"
>>   #include "qapi/qapi-events-migration.h"
>> +#include "qapi/qapi-commands-migration.h"
>>   #include "qapi/qmp/qerror.h"
>>   #include "trace.h"
>>   #include "exec/ram_addr.h"
>> @@ -57,6 +58,8 @@
>>   #include "qemu/iov.h"
>>   #include "multifd.h"
>>   #include "sysemu/runstate.h"
>> +#include "sysemu/dirtylimit.h"
>> +#include "sysemu/kvm.h"
>>   
>>   #include "hw/boards.h" /* for machine_dump_guest_core() */
>>   
>> @@ -1139,6 +1142,21 @@ static void migration_update_rates(RAMState *rs, int64_t end_time)
>>       }
>>   }
>>   
>> +/*
>> + * Enable dirty-limit to throttle down the guest
>> + */
>> +static void migration_dirty_limit_guest(void)
>> +{
>> +    if (!dirtylimit_in_service()) {
>> +        MigrationState *s = migrate_get_current();
>> +        int64_t quota_dirtyrate = s->parameters.x_vcpu_dirty_limit;
>> +
>> +        /* Set quota dirtyrate if dirty limit not in service */
>> +        qmp_set_vcpu_dirty_limit(false, -1, quota_dirtyrate, NULL);
>> +        trace_migration_dirty_limit_guest(quota_dirtyrate);
>> +    }
>> +}
>> +
>>   static void migration_trigger_throttle(RAMState *rs)
>>   {
>>       MigrationState *s = migrate_get_current();
>> @@ -1148,22 +1166,31 @@ static void migration_trigger_throttle(RAMState *rs)
>>       uint64_t bytes_dirty_period = rs->num_dirty_pages_period * TARGET_PAGE_SIZE;
>>       uint64_t bytes_dirty_threshold = bytes_xfer_period * threshold / 100;
>>   
>> -    /* During block migration the auto-converge logic incorrectly detects
>> -     * that ram migration makes no progress. Avoid this by disabling the
>> -     * throttling logic during the bulk phase of block migration. */
>> -    if (migrate_auto_converge() && !blk_mig_bulk_active()) {
>> -        /* The following detection logic can be refined later. For now:
>> -           Check to see if the ratio between dirtied bytes and the approx.
>> -           amount of bytes that just got transferred since the last time
>> -           we were in this routine reaches the threshold. If that happens
>> -           twice, start or increase throttling. */
>> -
>> -        if ((bytes_dirty_period > bytes_dirty_threshold) &&
>> -            (++rs->dirty_rate_high_cnt >= 2)) {
>> +    /*
>> +     * The following detection logic can be refined later. For now:
>> +     * Check to see if the ratio between dirtied bytes and the approx.
>> +     * amount of bytes that just got transferred since the last time
>> +     * we were in this routine reaches the threshold. If that happens
>> +     * twice, start or increase throttling.
>> +     */
>> +
>> +    if ((bytes_dirty_period > bytes_dirty_threshold) &&
>> +        (++rs->dirty_rate_high_cnt >= 2)) {
>> +        rs->dirty_rate_high_cnt = 0;
>> +        /*
>> +         * During block migration the auto-converge logic incorrectly detects
>> +         * that ram migration makes no progress. Avoid this by disabling the
>> +         * throttling logic during the bulk phase of block migration
>> +         */
>> +
>> +        if (migrate_auto_converge() && !blk_mig_bulk_active()) {
>>               trace_migration_throttle();
>> -            rs->dirty_rate_high_cnt = 0;
>>               mig_throttle_guest_down(bytes_dirty_period,
>>                                       bytes_dirty_threshold);
>> +        } else if (migrate_dirty_limit() &&
>> +                   kvm_dirty_ring_enabled() &&
>> +                   migration_is_active(s)) {
>> +            migration_dirty_limit_guest();
> 
> We'll call this multiple time, but only the 1st call will make sense, right?
Yes.
> 
> Can we call it once somewhere?  E.g. at the start of migration?It make sense indeed, if dirtylimit run once migration start, the 
behavior of dirtylimit migration would be kind of different from 
auto-converge, i mean, dirtylimit will make guest write vCPU slow no 
matter if dirty_rate_high_cnt exceed 2 times. For those vms that dirty 
memory lightly, they can get convergent without throttle, but in the new 
way ,if we set the dirtylimit to a very low value, they may suffer 
restriction. Can we accept that ?
> 
> Thanks,
>