Signed-off-by: Juan Quintela <quintela@redhat.com>
---
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;
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;
}
static void update_dirtyrate(uint64_t msec)
@@ -315,10 +316,10 @@ static void update_dirtyrate(uint64_t msec)
static uint32_t get_ramblock_vfn_hash(struct RamblockDirtyInfo *info,
uint64_t vfn)
{
+ int page_size = qemu_target_page_size();
uint32_t crc;
- crc = crc32(0, (info->ramblock_addr +
- vfn * TARGET_PAGE_SIZE), TARGET_PAGE_SIZE);
+ crc = crc32(0, info->ramblock_addr + vfn * page_size, page_size);
trace_get_ramblock_vfn_hash(info->idstr, vfn, crc);
return crc;
--
2.40.1On 5/11/23 10:22, Juan Quintela wrote: > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > 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. > > 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? r~
Richard Henderson <richard.henderson@linaro.org> wrote:
> On 5/11/23 10:22, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> 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.
Will resend the series on top of this.
Thanks, Juan.
On Thu, May 11, 2023 at 03:12:24PM +0200, Juan Quintela wrote:
> Richard Henderson <richard.henderson@linaro.org> wrote:
> > On 5/11/23 10:22, Juan Quintela wrote:
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >> 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.
Richard Henderson <richard.henderson@linaro.org> wrote: > On 5/11/23 10:22, Juan Quintela wrote: >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> 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. > >> 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? This is for dirtyrate folks. But I agree that having to do this so many times look wrong. I will wait to see if they answer. Later, Juan.
© 2016 - 2026 Red Hat, Inc.