Re: [Qemu-devel] [PATCH 4/4] migration: use dirty_rate_high_cnt more aggressively

[Felipe Franciosi]

> On 24 May 2017, at 17:25, Daniel P. Berrange <address@hidden> wrote:
> 
> On Wed, May 24, 2017 at 05:10:03PM +0100, Felipe Franciosi wrote:
>> The commit message from 070afca25 suggests that dirty_rate_high_cnt
>> should be used more aggressively to start throttling after two
>> iterations instead of four. The code, however, only changes the auto
>> convergence behaviour to throttle after three iterations. This makes the
>> behaviour more aggressive by kicking off throttling after two iterations
>> as originally intended.
> 
> This description looks suspect vs the code. You say it is changing
> from "after four" to "after two", but you are merely switching a
> post-increment to a pre-increment which can only reduce the boundary
> condition by 1, not 2. So either you mean to write
> 
> "after two iterations instead of three" 
> 
> or
> 
> "after three iterations intead of four"

Hi Daniel,

Thanks for the quick feedback. Let me clarify what I meant. If there's still 
confusion I'll update the commit message to reflect it better.

Pre-070afca25: throttling kicked in after four iterations.
Post-070afca25: throttling kicked in after three iterations (but Jason wrote he 
meant to start throttling after two).
This patch: throttles kicks in after two iterations (so Jason's code, minus 
one).

Perhaps where I said "The code, however," I could say "Commit 070afca25, 
however,". Let me know.

Thanks,
Felipe

> 
>> 
>> Signed-off-by: Felipe Franciosi <address@hidden>
>> ---
>> migration/ram.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 1a3d9e6..26e03a5 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -708,7 +708,7 @@ static void migration_bitmap_sync(RAMState *rs)
>> 
>>             if ((rs->num_dirty_pages_period * TARGET_PAGE_SIZE >
>>                    (bytes_xfer_now - rs->bytes_xfer_prev) / 2) &&
>> -                (rs->dirty_rate_high_cnt++ >= 2)) {
>> +                (++rs->dirty_rate_high_cnt >= 2)) {
>>                     trace_migration_throttle();
>>                     rs->dirty_rate_high_cnt = 0;
>>                     mig_throttle_guest_down();
>> -- 
>> 1.9.5
>> 
>> 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|