[PATCH 0/4] support dirtyrate measurement with dirty bitmap

huangy81@chinatelecom.cn posted 4 patches 2 years, 10 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
accel/kvm/kvm-all.c     |   6 ++
hmp-commands.hx         |   9 +--
include/exec/ram_addr.h | 140 +++++++++++++++++++++++++++++++++++++++++++++-
include/exec/ramlist.h  |   9 +--
include/sysemu/kvm.h    |   1 +
migration/dirtyrate.c   | 146 +++++++++++++++++++++++++++++++++++++++++++++---
migration/trace-events  |   2 +
qapi/migration.json     |   6 +-
softmmu/physmem.c       |  60 ++++++++++++++++++++
9 files changed, 358 insertions(+), 21 deletions(-)
[PATCH 0/4] support dirtyrate measurement with dirty bitmap
Posted by huangy81@chinatelecom.cn 2 years, 10 months ago
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

the dirtyrate measurement implemented by page-sampling originally, it
is not accurate in some scenarios, so we have introduced dirty-ring
based dirtyrate measurement(maybe it will be merged soon), it fix the
accuracy of page-sampling, and more importantly, it is at the
granualrity of vcpu.

dirty-ring method can be used when dirty-ring enable, as supplementary,
we introduce dirty-bitmap method to calculating dirtyrate when dirty log
enable, so that we can also get the accurate dirtyrate if needed in the
absence of dirty-ring.

three things has done to implement the measurement:
- introduce a fresh new dirty bits named DIRTY_MEMORY_DIRTY_RATE, which
  is used to store dirty bitmap after fetching it from kvm. why we do
  not reuse the existing DIRTY_MEMORY_MIGRATION dirty bits is we do not
  want to interfere with migration of and let implementation clear, this 
  is also the reason why dirty_memory be split.

  DIRTY_MEMORY_DIRTY_RATE dirty bits will be filled when
  memory_global_dirty_log_sync executed if GLOBAL_DIRTY_DIRTY_RATE bit
  be set in the global_dirty_tracking flag.

- introduce kvm_get_manual_dirty_log_protect function so that we can
  probe the protect caps of kvm when calculating.

- implement dirtyrate measurement with dirty bitmap with following step:
  1. start the dirty log. 

  2. probe the protect cap, if KVM_DIRTY_LOG_INITIALLY_SET enable, skip
     skip the 1'R and do the reset page protection manually, since kvm
     file bitmap with 1 bits if this cap is enabled. 

  3. clear the DIRTY_MEMORY_DIRTY_RATE dirty bits, prepare to store 
     the dirty bitmap.

  4. start memory_global_dirty_log_sync and fetch dirty bitmap from kvm

  5. reap the DIRTY_MEMORY_DIRTY_RATE dirty bits and do the calculation.

this patchset rebases on the commit 
"migration/dirtyrate: implement dirty-ring dirtyrate calculation",
since the above feature has not been merged, so we post this patch
for the sake of RFC. ideally, this patshset may be merged after it.

Please, review, thanks !

Best Regards !

Hyman Huang(黄勇) (4):
  memory: introduce DIRTY_MEMORY_DIRTY_RATE dirty bits
  KVM: introduce kvm_get_manual_dirty_log_protect
  memory: introduce DIRTY_MEMORY_DIRTY_RATE dirty bits functions
  migration/dirtyrate: implement dirty-bitmap dirtyrate calculation

 accel/kvm/kvm-all.c     |   6 ++
 hmp-commands.hx         |   9 +--
 include/exec/ram_addr.h | 140 +++++++++++++++++++++++++++++++++++++++++++++-
 include/exec/ramlist.h  |   9 +--
 include/sysemu/kvm.h    |   1 +
 migration/dirtyrate.c   | 146 +++++++++++++++++++++++++++++++++++++++++++++---
 migration/trace-events  |   2 +
 qapi/migration.json     |   6 +-
 softmmu/physmem.c       |  60 ++++++++++++++++++++
 9 files changed, 358 insertions(+), 21 deletions(-)

-- 
1.8.3.1


Re: [PATCH 0/4] support dirtyrate measurement with dirty bitmap
Posted by Peter Xu 2 years, 10 months ago
Yong,

On Sun, Jun 27, 2021 at 01:38:13PM +0800, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> the dirtyrate measurement implemented by page-sampling originally, it
> is not accurate in some scenarios, so we have introduced dirty-ring
> based dirtyrate measurement(maybe it will be merged soon), it fix the
> accuracy of page-sampling, and more importantly, it is at the
> granualrity of vcpu.
> 
> dirty-ring method can be used when dirty-ring enable, as supplementary,
> we introduce dirty-bitmap method to calculating dirtyrate when dirty log
> enable, so that we can also get the accurate dirtyrate if needed in the
> absence of dirty-ring.
> 
> three things has done to implement the measurement:
> - introduce a fresh new dirty bits named DIRTY_MEMORY_DIRTY_RATE, which
>   is used to store dirty bitmap after fetching it from kvm. why we do
>   not reuse the existing DIRTY_MEMORY_MIGRATION dirty bits is we do not
>   want to interfere with migration of and let implementation clear, this 
>   is also the reason why dirty_memory be split.
> 
>   DIRTY_MEMORY_DIRTY_RATE dirty bits will be filled when
>   memory_global_dirty_log_sync executed if GLOBAL_DIRTY_DIRTY_RATE bit
>   be set in the global_dirty_tracking flag.

I'm not 100% sure this is needed.

Dirty rate measurements do not care about which page is dirtied, it looks like
an overkill to introduce a new bitmap for it.

IMHO we can directly do the calculation when synchronizing the dirty bits in
below functions:

        cpu_physical_memory_set_dirty_range
        cpu_physical_memory_set_dirty_lebitmap
        cpu_physical_memory_sync_dirty_bitmap

Maybe we can define a global statistics for that?

> 
> - introduce kvm_get_manual_dirty_log_protect function so that we can
>   probe the protect caps of kvm when calculating.
> 
> - implement dirtyrate measurement with dirty bitmap with following step:
>   1. start the dirty log. 
> 
>   2. probe the protect cap, if KVM_DIRTY_LOG_INITIALLY_SET enable, skip
>      skip the 1'R and do the reset page protection manually, since kvm
>      file bitmap with 1 bits if this cap is enabled. 
> 
>   3. clear the DIRTY_MEMORY_DIRTY_RATE dirty bits, prepare to store 
>      the dirty bitmap.
> 
>   4. start memory_global_dirty_log_sync and fetch dirty bitmap from kvm
> 
>   5. reap the DIRTY_MEMORY_DIRTY_RATE dirty bits and do the calculation.
> 
> this patchset rebases on the commit 
> "migration/dirtyrate: implement dirty-ring dirtyrate calculation",
> since the above feature has not been merged, so we post this patch
> for the sake of RFC. ideally, this patshset may be merged after it.

I gave it a shot with some setup dirty workload, it runs well so far and also I
do get accurate numbers (200MB/s measured as 201MB/s; 300MB/s measured as
301MB/s, and so on).  Looks good to me in general.

But as I mentioned above I feel like the changeset can be shrinked quite a bit
if we can drop the extra bitmap; maybe it means we can drop half of the whole
series.  But it's also possible I missed something, let's see.

It'll slightly differ from dirty ring in that same page written will always
only be counted once between two dirty map sync, but that's expected.  Dirty
ring "sync" more frequently (either ring full, or current 1-sec timeout in the
reaper), so it re-protects more frequently too.

I still have some other small comments, I'll go into the patches.

Thanks,

-- 
Peter Xu


Re: [PATCH 0/4] support dirtyrate measurement with dirty bitmap
Posted by Hyman Huang 2 years, 9 months ago

在 2021/7/10 2:20, Peter Xu 写道:
> Yong,
> 
> On Sun, Jun 27, 2021 at 01:38:13PM +0800, huangy81@chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> the dirtyrate measurement implemented by page-sampling originally, it
>> is not accurate in some scenarios, so we have introduced dirty-ring
>> based dirtyrate measurement(maybe it will be merged soon), it fix the
>> accuracy of page-sampling, and more importantly, it is at the
>> granualrity of vcpu.
>>
>> dirty-ring method can be used when dirty-ring enable, as supplementary,
>> we introduce dirty-bitmap method to calculating dirtyrate when dirty log
>> enable, so that we can also get the accurate dirtyrate if needed in the
>> absence of dirty-ring.
>>
>> three things has done to implement the measurement:
>> - introduce a fresh new dirty bits named DIRTY_MEMORY_DIRTY_RATE, which
>>    is used to store dirty bitmap after fetching it from kvm. why we do
>>    not reuse the existing DIRTY_MEMORY_MIGRATION dirty bits is we do not
>>    want to interfere with migration of and let implementation clear, this
>>    is also the reason why dirty_memory be split.
>>
>>    DIRTY_MEMORY_DIRTY_RATE dirty bits will be filled when
>>    memory_global_dirty_log_sync executed if GLOBAL_DIRTY_DIRTY_RATE bit
>>    be set in the global_dirty_tracking flag.
> 
> I'm not 100% sure this is needed.
> 
> Dirty rate measurements do not care about which page is dirtied, it looks like
> an overkill to introduce a new bitmap for it.
indeed, dirty rate measurements only cares about the increased dirty 
pages number during calculation time.
> 
> IMHO we can directly do the calculation when synchronizing the dirty bits in
> below functions:
> 
>          cpu_physical_memory_set_dirty_range
>          cpu_physical_memory_set_dirty_lebitmap
>          cpu_physical_memory_sync_dirty_bitmap
> 
> Maybe we can define a global statistics for that?
uhhh... Do you mean that we can reuse the DIRTY_MEMORY_MIGRATION dirty 
bits to stat the new dirty pages number and just define the global var 
to count the increased dirty pages during the calculation time?
or we still use the bitmap but defined as a global var, instead of dirty 
bits?
> 
>>
>> - introduce kvm_get_manual_dirty_log_protect function so that we can
>>    probe the protect caps of kvm when calculating.
>>
>> - implement dirtyrate measurement with dirty bitmap with following step:
>>    1. start the dirty log.
>>
>>    2. probe the protect cap, if KVM_DIRTY_LOG_INITIALLY_SET enable, skip
>>       skip the 1'R and do the reset page protection manually, since kvm
>>       file bitmap with 1 bits if this cap is enabled.
>>
>>    3. clear the DIRTY_MEMORY_DIRTY_RATE dirty bits, prepare to store
>>       the dirty bitmap.
>>
>>    4. start memory_global_dirty_log_sync and fetch dirty bitmap from kvm
>>
>>    5. reap the DIRTY_MEMORY_DIRTY_RATE dirty bits and do the calculation.
>>
>> this patchset rebases on the commit
>> "migration/dirtyrate: implement dirty-ring dirtyrate calculation",
>> since the above feature has not been merged, so we post this patch
>> for the sake of RFC. ideally, this patshset may be merged after it.
> 
> I gave it a shot with some setup dirty workload, it runs well so far and also I
> do get accurate numbers (200MB/s measured as 201MB/s; 300MB/s measured as
> 301MB/s, and so on).  Looks good to me in general.
> 
> But as I mentioned above I feel like the changeset can be shrinked quite a bit
> if we can drop the extra bitmap; maybe it means we can drop half of the whole
> series.  But it's also possible I missed something, let's see.
> 
> It'll slightly differ from dirty ring in that same page written will always
> only be counted once between two dirty map sync, but that's expected.  Dirty
> ring "sync" more frequently (either ring full, or current 1-sec timeout in the
> reaper), so it re-protects more frequently too.
> 
> I still have some other small comments, I'll go into the patches.
> 
> Thanks,
> 

-- 
Best regard

Hyman Huang(黄勇)

Re: [PATCH 0/4] support dirtyrate measurement with dirty bitmap
Posted by Peter Xu 2 years, 9 months ago
On Sun, Jul 11, 2021 at 11:27:13PM +0800, Hyman Huang wrote:
> > IMHO we can directly do the calculation when synchronizing the dirty bits in
> > below functions:
> > 
> >          cpu_physical_memory_set_dirty_range
> >          cpu_physical_memory_set_dirty_lebitmap
> >          cpu_physical_memory_sync_dirty_bitmap
> > 
> > Maybe we can define a global statistics for that?
> uhhh... Do you mean that we can reuse the DIRTY_MEMORY_MIGRATION dirty bits
> to stat the new dirty pages number and just define the global var to count
> the increased dirty pages during the calculation time?

I think I misguided you.. Sorry :)

cpu_physical_memory_sync_dirty_bitmap() should not really be in the list above,
as it's fetching the bitmap in ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION].

If you see the other two functions, they all apply dirty bits upon the same
bitmap (actually ram_list.dirty_memory[*] instead of migration-only).  It's
used by e.g. memory region log_sync() to deliver lower level dirty bits to
upper, e.g., see kvm's log_sync[_global]() and kvm_slot_sync_dirty_pages().

Using cpu_physical_memory_sync_dirty_bitmap() is not a good idea to me (which I
saw you used in your latest version), as it could affect migration.  See its
only caller now at ramblock_sync_dirty_bitmap(): when migration calls it, it'll
start to count less than it should for rs->migration_dirty_pages.

So what I wanted to suggest is we do some general counting in both
cpu_physical_memory_set_dirty_range and cpu_physical_memory_set_dirty_lebitmap.
Then to sync for dirty rate measuring, we use memory_global_dirty_log_sync().
That'll sync all dirty bits e.g. in kernel to ram_list.dirty_memory[*], along
which we do the accounting.

Would that work?

-- 
Peter Xu


Re: [PATCH 0/4] support dirtyrate measurement with dirty bitmap
Posted by Hyman 2 years, 9 months ago

在 2021/7/14 1:45, Peter Xu 写道:
> On Sun, Jul 11, 2021 at 11:27:13PM +0800, Hyman Huang wrote:
>>> IMHO we can directly do the calculation when synchronizing the dirty bits in
>>> below functions:
>>>
>>>           cpu_physical_memory_set_dirty_range
>>>           cpu_physical_memory_set_dirty_lebitmap
>>>           cpu_physical_memory_sync_dirty_bitmap
>>>
>>> Maybe we can define a global statistics for that?
>> uhhh... Do you mean that we can reuse the DIRTY_MEMORY_MIGRATION dirty bits
>> to stat the new dirty pages number and just define the global var to count
>> the increased dirty pages during the calculation time?
> 
> I think I misguided you.. Sorry :)
never mind, the other version of the implementation is what your said, 
i'll post later.
> 
> cpu_physical_memory_sync_dirty_bitmap() should not really be in the list above,
> as it's fetching the bitmap in ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION].
> 
> If you see the other two functions, they all apply dirty bits upon the same
> bitmap (actually ram_list.dirty_memory[*] instead of migration-only).  It's
> used by e.g. memory region log_sync() to deliver lower level dirty bits to
> upper, e.g., see kvm's log_sync[_global]() and kvm_slot_sync_dirty_pages().
> 
> Using cpu_physical_memory_sync_dirty_bitmap() is not a good idea to me (which I
> saw you used in your latest version), as it could affect migration.  See its
> only caller now at ramblock_sync_dirty_bitmap(): when migration calls it, it'll
> start to count less than it should for rs->migration_dirty_pages.
> 
> So what I wanted to suggest is we do some general counting in both
> cpu_physical_memory_set_dirty_range and cpu_physical_memory_set_dirty_lebitmap.
> Then to sync for dirty rate measuring, we use memory_global_dirty_log_sync().
> That'll sync all dirty bits e.g. in kernel to ram_list.dirty_memory[*], along
> which we do the accounting.
> 
> Would that work?
yes, this works.
>