All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chao Fan <fanc.fnst@cn.fujitsu.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: dgilbert@redhat.com, qemu-devel@nongnu.org, quintela@redhat.com,
	eblake@redhat.com, armbru@redhat.com, douly.fnst@cn.fujitsu.com,
	caoj.fnst@cn.fujitsu.com, maozy.fnst@cn.fujitsu.com,
	lizhijian@cn.fujitsu.com
Subject: Re: [Qemu-devel] [PATCH v2] Add inst_dirty_pages_rate in 'info migrate'
Date: Thu, 9 Mar 2017 18:05:38 +0800	[thread overview]
Message-ID: <20170309100538.GA22103@localhost.localdomain> (raw)
In-Reply-To: <20170308134559.GK7470@redhat.com>

On Wed, Mar 08, 2017 at 01:45:59PM +0000, Daniel P. Berrange wrote:
>On Wed, Mar 08, 2017 at 04:28:19PM +0800, Chao Fan wrote:
>> Auto-converge aims to accelerate migration by slowing down the
>> generation of dirty pages. But user doesn't know how to determine the
>> throttle value, so, a new item "inst-dirty-pages-rate" in "info migrate"
>> would be helpful for user's determination.
Hi Daniel,

Thank you for your reply.
>
>The "info migrate" command already reports a "dirty-pages-rate" value.
>
>Maybe I'm mis-understanding what you're calculcating, this this proposal
>looks the same, except reporting in bytes rather than page counts.
>
>QEMU in fact already records the bytes count internally too in the
>'dirty_pages_bytes' parameter which is calculated from taking
>'dirty_pages_size * TARGET_PAGE_SIZE'.
>
>So I wonder if we can just export the existing dirty-pages-bytes
>value in info migrate, and avoid needing this new code here:
>
It's different, inst-dirty-pages-rate in this patch is greater than
or equal to dirty-pages-bytes. Because in function
cpu_physical_memory_sync_dirty_bitmap, file include/exec/ram_addr.h:

if (src[idx][offset]) {
    unsigned long bits = atomic_xchg(&src[idx][offset], 0);
    unsigned long new_dirty;
    new_dirty = ~dest[k];
    dest[k] |= bits;
    new_dirty &= bits;
    num_dirty += ctpopl(new_dirty);
}

After these codes, only the pages not dirtied in bitmap(dest), but dirtied
in dirty_memory[DIRTY_MEMORY_MIGRATION] will be calculated. For example:
When ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] = 0b00001111,
and atomic_rcu_read(&migration_bitmap_rcu)->bmap = 0b00000011,
the new_dirty will be 0b00001100, and this function will return 2 but not
4 which is expected.

Thanks,
Chao Fan

>> +static void migration_inst_rate(void)
>> +{
>> +    int64_t dirty_pages_time_now;
>> +    if (!dirty_pages_time_prev) {
>> +        dirty_pages_time_prev = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>> +    }
>> +    dirty_pages_time_now = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>> +    if (dirty_pages_time_now > dirty_pages_time_prev + 1000) {
>> +        RAMBlock *block;
>> +        MigrationState *s = migrate_get_current();
>> +        int64_t inst_dirty_pages = 0;
>> +        int64_t i;
>> +        unsigned long *num;
>> +        unsigned long len = 0;
>> +
>> +        rcu_read_lock();
>> +        DirtyMemoryBlocks *blocks = atomic_rcu_read(
>> +                         &ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]);
>> +        QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
>> +            if (len == 0) {
>> +                len = block->offset;
>> +            }
>> +            len += block->used_length;
>> +        }
>> +        ram_addr_t idx = (len >> TARGET_PAGE_BITS) / DIRTY_MEMORY_BLOCK_SIZE;
>> +        if (((len >> TARGET_PAGE_BITS) % DIRTY_MEMORY_BLOCK_SIZE) != 0) {
>> +            idx++;
>> +        }
>> +        for (i = 0; i < idx; i++) {
>> +            num = blocks->blocks[i];
>> +            inst_dirty_pages += bitmap_weight(num, DIRTY_MEMORY_BLOCK_SIZE);
>> +        }
>> +        rcu_read_unlock();
>> +
>> +        s->inst_dirty_pages_rate = inst_dirty_pages * TARGET_PAGE_SIZE *
>> +                    1000 / (dirty_pages_time_now - dirty_pages_time_prev);
>> +    }
>> +    dirty_pages_time_prev = dirty_pages_time_now;
>>  }
>
>Regards,
>Daniel
>-- 
>|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
>|: http://libvirt.org              -o-             http://virt-manager.org :|
>|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|
>
>

  reply	other threads:[~2017-03-09 10:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-08  8:28 [Qemu-devel] [PATCH v2] Add inst_dirty_pages_rate in 'info migrate' Chao Fan
2017-03-08 13:45 ` Daniel P. Berrange
2017-03-09 10:05   ` Chao Fan [this message]
2017-03-10 13:31     ` Daniel P. Berrange

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170309100538.GA22103@localhost.localdomain \
    --to=fanc.fnst@cn.fujitsu.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=caoj.fnst@cn.fujitsu.com \
    --cc=dgilbert@redhat.com \
    --cc=douly.fnst@cn.fujitsu.com \
    --cc=eblake@redhat.com \
    --cc=lizhijian@cn.fujitsu.com \
    --cc=maozy.fnst@cn.fujitsu.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.