On Thu, May 11, 2023 at 03:12:24PM +0200, Juan Quintela wrote: > Richard Henderson <[email protected]> wrote: > > On 5/11/23 10:22, Juan Quintela wrote: > >> Signed-off-by: Juan Quintela <[email protected]> > >> --- > >> migration/dirtyrate.c | 11 ++++++----- > >> 1 file changed, 6 insertions(+), 5 deletions(-) > >> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c > >> index 180ba38c7a..9aa092738c 100644 > >> --- a/migration/dirtyrate.c > >> +++ b/migration/dirtyrate.c > >> @@ -17,6 +17,7 @@ > >> #include "cpu.h" > >> #include "exec/ramblock.h" > >> #include "exec/ram_addr.h" > >> +#include "exec/target_page.h" > >> #include "qemu/rcu_queue.h" > >> #include "qemu/main-loop.h" > >> #include "qapi/qapi-commands-migration.h" > >> @@ -78,7 +79,7 @@ static int64_t do_calculate_dirtyrate(DirtyPageRecord > >> dirty_pages, > >> uint64_t increased_dirty_pages = > >> dirty_pages.end_pages - dirty_pages.start_pages; > >> - memory_size_MB = (increased_dirty_pages * TARGET_PAGE_SIZE) > >> >> 20; > >> + memory_size_MB = (increased_dirty_pages * qemu_target_page_size()) >> > >> 20; > > > > See the recent cleanups for dirtylimit_dirty_ring_full_time, folding > > multiply+shift into subtract+shift. > > I reviewed it and I had already forgotten!! > > >> return memory_size_MB * 1000 / calc_time_ms; > >> } > >> @@ -291,8 +292,8 @@ static void update_dirtyrate_stat(struct > >> RamblockDirtyInfo *info) > >> DirtyStat.page_sampling.total_dirty_samples += > >> info->sample_dirty_count; > >> DirtyStat.page_sampling.total_sample_count += > >> info->sample_pages_count; > >> /* size of total pages in MB */ > >> - DirtyStat.page_sampling.total_block_mem_MB += (info->ramblock_pages * > >> - TARGET_PAGE_SIZE) >> > >> 20; > >> + DirtyStat.page_sampling.total_block_mem_MB += > >> + (info->ramblock_pages * qemu_target_page_size()) >> 20; > > > > And a third copy? > > Can we abstract this somewhere? > > I ended with this: > > /* Convert target pages to MiB (2**20). */ > size_t qemu_target_pages_to_MiB(size_t pages) > { > int page_bits = TARGET_PAGE_BITS; > > /* So far, the largest (non-huge) page size is 64k, i.e. 16 bits. */ > g_assert(page_bits < 20); > > return pages >> (20 - page_bits); > } > > But only found 3 users, the one that you did and this two.
In fact, there is no need to convert VM size into MB inside update_dirtyrate_stat(). The unit of total_block_mem_MB can be changed to pages (or bytes). This will leave update_dirtyrate() as the only user of the proposed function. > > Will resend the series on top of this. > > Thanks, Juan.
