| Message ID | 20170323204544.12015-41-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
* Juan Quintela (quintela@redhat.com) wrote: > It was used as a size in all cases except one. > > Signed-off-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > exec.c | 4 ++-- > include/sysemu/sysemu.h | 2 +- > migration/migration.c | 4 ++-- > migration/postcopy-ram.c | 8 ++++---- > migration/savevm.c | 8 ++++---- > 5 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/exec.c b/exec.c > index e57a8a2..9a4c385 100644 > --- a/exec.c > +++ b/exec.c > @@ -3349,9 +3349,9 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > * Allows code that needs to deal with migration bitmaps etc to still be built > * target independent. > */ > -size_t qemu_target_page_bits(void) > +size_t qemu_target_page_size(void) > { > - return TARGET_PAGE_BITS; > + return TARGET_PAGE_SIZE; > } > > #endif > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 576c7ce..16175f7 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -67,7 +67,7 @@ int qemu_reset_requested_get(void); > void qemu_system_killed(int signal, pid_t pid); > void qemu_system_reset(bool report); > void qemu_system_guest_panicked(GuestPanicInformation *info); > -size_t qemu_target_page_bits(void); > +size_t qemu_target_page_size(void); > > void qemu_add_exit_notifier(Notifier *notify); > void qemu_remove_exit_notifier(Notifier *notify); > diff --git a/migration/migration.c b/migration/migration.c > index 3f99ab3..92c3c6b 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -646,7 +646,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) > info->ram->skipped = 0; > info->ram->normal = norm_mig_pages_transferred(); > info->ram->normal_bytes = norm_mig_pages_transferred() * > - (1ul << qemu_target_page_bits()); > + qemu_target_page_size(); > info->ram->mbps = s->mbps; > info->ram->dirty_sync_count = ram_dirty_sync_count(); > info->ram->postcopy_requests = ram_postcopy_requests(); > @@ -2001,7 +2001,7 @@ static void *migration_thread(void *opaque) > 10000 is a small enough number for our purposes */ > if (ram_dirty_pages_rate() && transferred_bytes > 10000) { > s->expected_downtime = ram_dirty_pages_rate() * > - (1ul << qemu_target_page_bits()) / bandwidth; > + qemu_target_page_size() / bandwidth; > } > > qemu_file_reset_rate_limit(s->to_dst_file); > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > index dc80dbb..8756364 100644 > --- a/migration/postcopy-ram.c > +++ b/migration/postcopy-ram.c > @@ -123,7 +123,7 @@ bool postcopy_ram_supported_by_host(void) > struct uffdio_range range_struct; > uint64_t feature_mask; > > - if ((1ul << qemu_target_page_bits()) > pagesize) { > + if (qemu_target_page_size() > pagesize) { > error_report("Target page size bigger than host page size"); > goto out; > } > @@ -745,10 +745,10 @@ PostcopyDiscardState *postcopy_discard_send_init(MigrationState *ms, > void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState *pds, > unsigned long start, unsigned long length) > { > - size_t tp_bits = qemu_target_page_bits(); > + size_t tp_size = qemu_target_page_size(); > /* Convert to byte offsets within the RAM block */ > - pds->start_list[pds->cur_entry] = (start - pds->offset) << tp_bits; > - pds->length_list[pds->cur_entry] = length << tp_bits; > + pds->start_list[pds->cur_entry] = (start - pds->offset) * tp_size; > + pds->length_list[pds->cur_entry] = length * tp_size; > trace_postcopy_discard_send_range(pds->ramblock_name, start, length); > pds->cur_entry++; > pds->nsentwords++; > diff --git a/migration/savevm.c b/migration/savevm.c > index 853a81a..bbf055d 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -871,7 +871,7 @@ void qemu_savevm_send_postcopy_advise(QEMUFile *f) > { > uint64_t tmp[2]; > tmp[0] = cpu_to_be64(ram_pagesize_summary()); > - tmp[1] = cpu_to_be64(1ul << qemu_target_page_bits()); > + tmp[1] = cpu_to_be64(qemu_target_page_size()); > > trace_qemu_savevm_send_postcopy_advise(); > qemu_savevm_command_send(f, MIG_CMD_POSTCOPY_ADVISE, 16, (uint8_t *)tmp); > @@ -1390,13 +1390,13 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis) > } > > remote_tps = qemu_get_be64(mis->from_src_file); > - if (remote_tps != (1ul << qemu_target_page_bits())) { > + if (remote_tps != qemu_target_page_size()) { > /* > * Again, some differences could be dealt with, but for now keep it > * simple. > */ > - error_report("Postcopy needs matching target page sizes (s=%d d=%d)", > - (int)remote_tps, 1 << qemu_target_page_bits()); > + error_report("Postcopy needs matching target page sizes (s=%d d=%zd)", > + (int)remote_tps, qemu_target_page_size()); > return -1; > } > > -- > 2.9.3 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Thu, Mar 23, 2017 at 09:45:33PM +0100, Juan Quintela wrote: > It was used as a size in all cases except one. Considering that: - qemu_target_page_bits() is only used in migration codes, in only several places below - migration codes is using TARGET_PAGE_{BITS|SIZE} a lot as well How about we just remove this function, and directly use TARGET_PAGE_{BITS|SIZE}? Thanks, > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > exec.c | 4 ++-- > include/sysemu/sysemu.h | 2 +- > migration/migration.c | 4 ++-- > migration/postcopy-ram.c | 8 ++++---- > migration/savevm.c | 8 ++++---- > 5 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/exec.c b/exec.c > index e57a8a2..9a4c385 100644 > --- a/exec.c > +++ b/exec.c > @@ -3349,9 +3349,9 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > * Allows code that needs to deal with migration bitmaps etc to still be built > * target independent. > */ > -size_t qemu_target_page_bits(void) > +size_t qemu_target_page_size(void) > { > - return TARGET_PAGE_BITS; > + return TARGET_PAGE_SIZE; > } > > #endif > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 576c7ce..16175f7 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -67,7 +67,7 @@ int qemu_reset_requested_get(void); > void qemu_system_killed(int signal, pid_t pid); > void qemu_system_reset(bool report); > void qemu_system_guest_panicked(GuestPanicInformation *info); > -size_t qemu_target_page_bits(void); > +size_t qemu_target_page_size(void); > > void qemu_add_exit_notifier(Notifier *notify); > void qemu_remove_exit_notifier(Notifier *notify); > diff --git a/migration/migration.c b/migration/migration.c > index 3f99ab3..92c3c6b 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -646,7 +646,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) > info->ram->skipped = 0; > info->ram->normal = norm_mig_pages_transferred(); > info->ram->normal_bytes = norm_mig_pages_transferred() * > - (1ul << qemu_target_page_bits()); > + qemu_target_page_size(); > info->ram->mbps = s->mbps; > info->ram->dirty_sync_count = ram_dirty_sync_count(); > info->ram->postcopy_requests = ram_postcopy_requests(); > @@ -2001,7 +2001,7 @@ static void *migration_thread(void *opaque) > 10000 is a small enough number for our purposes */ > if (ram_dirty_pages_rate() && transferred_bytes > 10000) { > s->expected_downtime = ram_dirty_pages_rate() * > - (1ul << qemu_target_page_bits()) / bandwidth; > + qemu_target_page_size() / bandwidth; > } > > qemu_file_reset_rate_limit(s->to_dst_file); > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > index dc80dbb..8756364 100644 > --- a/migration/postcopy-ram.c > +++ b/migration/postcopy-ram.c > @@ -123,7 +123,7 @@ bool postcopy_ram_supported_by_host(void) > struct uffdio_range range_struct; > uint64_t feature_mask; > > - if ((1ul << qemu_target_page_bits()) > pagesize) { > + if (qemu_target_page_size() > pagesize) { > error_report("Target page size bigger than host page size"); > goto out; > } > @@ -745,10 +745,10 @@ PostcopyDiscardState *postcopy_discard_send_init(MigrationState *ms, > void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState *pds, > unsigned long start, unsigned long length) > { > - size_t tp_bits = qemu_target_page_bits(); > + size_t tp_size = qemu_target_page_size(); > /* Convert to byte offsets within the RAM block */ > - pds->start_list[pds->cur_entry] = (start - pds->offset) << tp_bits; > - pds->length_list[pds->cur_entry] = length << tp_bits; > + pds->start_list[pds->cur_entry] = (start - pds->offset) * tp_size; > + pds->length_list[pds->cur_entry] = length * tp_size; > trace_postcopy_discard_send_range(pds->ramblock_name, start, length); > pds->cur_entry++; > pds->nsentwords++; > diff --git a/migration/savevm.c b/migration/savevm.c > index 853a81a..bbf055d 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -871,7 +871,7 @@ void qemu_savevm_send_postcopy_advise(QEMUFile *f) > { > uint64_t tmp[2]; > tmp[0] = cpu_to_be64(ram_pagesize_summary()); > - tmp[1] = cpu_to_be64(1ul << qemu_target_page_bits()); > + tmp[1] = cpu_to_be64(qemu_target_page_size()); > > trace_qemu_savevm_send_postcopy_advise(); > qemu_savevm_command_send(f, MIG_CMD_POSTCOPY_ADVISE, 16, (uint8_t *)tmp); > @@ -1390,13 +1390,13 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis) > } > > remote_tps = qemu_get_be64(mis->from_src_file); > - if (remote_tps != (1ul << qemu_target_page_bits())) { > + if (remote_tps != qemu_target_page_size()) { > /* > * Again, some differences could be dealt with, but for now keep it > * simple. > */ > - error_report("Postcopy needs matching target page sizes (s=%d d=%d)", > - (int)remote_tps, 1 << qemu_target_page_bits()); > + error_report("Postcopy needs matching target page sizes (s=%d d=%zd)", > + (int)remote_tps, qemu_target_page_size()); > return -1; > } > > -- > 2.9.3 > > -- peterx
* Peter Xu (peterx@redhat.com) wrote: > On Thu, Mar 23, 2017 at 09:45:33PM +0100, Juan Quintela wrote: > > It was used as a size in all cases except one. > > Considering that: > > - qemu_target_page_bits() is only used in migration codes, in only > several places below > > - migration codes is using TARGET_PAGE_{BITS|SIZE} a lot as well > > How about we just remove this function, and directly use > TARGET_PAGE_{BITS|SIZE}? We can't because the TARGET_* macros are defined in headers that are only allowed to be included in target-specific builds. Most of QEMUs code (including all migration/*) is built target independent and so the headers error if we try and include them. That's why I added qemu_target_page_bits() Dave > Thanks, > > > > > Signed-off-by: Juan Quintela <quintela@redhat.com> > > --- > > exec.c | 4 ++-- > > include/sysemu/sysemu.h | 2 +- > > migration/migration.c | 4 ++-- > > migration/postcopy-ram.c | 8 ++++---- > > migration/savevm.c | 8 ++++---- > > 5 files changed, 13 insertions(+), 13 deletions(-) > > > > diff --git a/exec.c b/exec.c > > index e57a8a2..9a4c385 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -3349,9 +3349,9 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, > > * Allows code that needs to deal with migration bitmaps etc to still be built > > * target independent. > > */ > > -size_t qemu_target_page_bits(void) > > +size_t qemu_target_page_size(void) > > { > > - return TARGET_PAGE_BITS; > > + return TARGET_PAGE_SIZE; > > } > > > > #endif > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > > index 576c7ce..16175f7 100644 > > --- a/include/sysemu/sysemu.h > > +++ b/include/sysemu/sysemu.h > > @@ -67,7 +67,7 @@ int qemu_reset_requested_get(void); > > void qemu_system_killed(int signal, pid_t pid); > > void qemu_system_reset(bool report); > > void qemu_system_guest_panicked(GuestPanicInformation *info); > > -size_t qemu_target_page_bits(void); > > +size_t qemu_target_page_size(void); > > > > void qemu_add_exit_notifier(Notifier *notify); > > void qemu_remove_exit_notifier(Notifier *notify); > > diff --git a/migration/migration.c b/migration/migration.c > > index 3f99ab3..92c3c6b 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -646,7 +646,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) > > info->ram->skipped = 0; > > info->ram->normal = norm_mig_pages_transferred(); > > info->ram->normal_bytes = norm_mig_pages_transferred() * > > - (1ul << qemu_target_page_bits()); > > + qemu_target_page_size(); > > info->ram->mbps = s->mbps; > > info->ram->dirty_sync_count = ram_dirty_sync_count(); > > info->ram->postcopy_requests = ram_postcopy_requests(); > > @@ -2001,7 +2001,7 @@ static void *migration_thread(void *opaque) > > 10000 is a small enough number for our purposes */ > > if (ram_dirty_pages_rate() && transferred_bytes > 10000) { > > s->expected_downtime = ram_dirty_pages_rate() * > > - (1ul << qemu_target_page_bits()) / bandwidth; > > + qemu_target_page_size() / bandwidth; > > } > > > > qemu_file_reset_rate_limit(s->to_dst_file); > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > > index dc80dbb..8756364 100644 > > --- a/migration/postcopy-ram.c > > +++ b/migration/postcopy-ram.c > > @@ -123,7 +123,7 @@ bool postcopy_ram_supported_by_host(void) > > struct uffdio_range range_struct; > > uint64_t feature_mask; > > > > - if ((1ul << qemu_target_page_bits()) > pagesize) { > > + if (qemu_target_page_size() > pagesize) { > > error_report("Target page size bigger than host page size"); > > goto out; > > } > > @@ -745,10 +745,10 @@ PostcopyDiscardState *postcopy_discard_send_init(MigrationState *ms, > > void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState *pds, > > unsigned long start, unsigned long length) > > { > > - size_t tp_bits = qemu_target_page_bits(); > > + size_t tp_size = qemu_target_page_size(); > > /* Convert to byte offsets within the RAM block */ > > - pds->start_list[pds->cur_entry] = (start - pds->offset) << tp_bits; > > - pds->length_list[pds->cur_entry] = length << tp_bits; > > + pds->start_list[pds->cur_entry] = (start - pds->offset) * tp_size; > > + pds->length_list[pds->cur_entry] = length * tp_size; > > trace_postcopy_discard_send_range(pds->ramblock_name, start, length); > > pds->cur_entry++; > > pds->nsentwords++; > > diff --git a/migration/savevm.c b/migration/savevm.c > > index 853a81a..bbf055d 100644 > > --- a/migration/savevm.c > > +++ b/migration/savevm.c > > @@ -871,7 +871,7 @@ void qemu_savevm_send_postcopy_advise(QEMUFile *f) > > { > > uint64_t tmp[2]; > > tmp[0] = cpu_to_be64(ram_pagesize_summary()); > > - tmp[1] = cpu_to_be64(1ul << qemu_target_page_bits()); > > + tmp[1] = cpu_to_be64(qemu_target_page_size()); > > > > trace_qemu_savevm_send_postcopy_advise(); > > qemu_savevm_command_send(f, MIG_CMD_POSTCOPY_ADVISE, 16, (uint8_t *)tmp); > > @@ -1390,13 +1390,13 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis) > > } > > > > remote_tps = qemu_get_be64(mis->from_src_file); > > - if (remote_tps != (1ul << qemu_target_page_bits())) { > > + if (remote_tps != qemu_target_page_size()) { > > /* > > * Again, some differences could be dealt with, but for now keep it > > * simple. > > */ > > - error_report("Postcopy needs matching target page sizes (s=%d d=%d)", > > - (int)remote_tps, 1 << qemu_target_page_bits()); > > + error_report("Postcopy needs matching target page sizes (s=%d d=%zd)", > > + (int)remote_tps, qemu_target_page_size()); > > return -1; > > } > > > > -- > > 2.9.3 > > > > > > -- peterx -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Peter Xu <peterx@redhat.com> wrote: > On Thu, Mar 23, 2017 at 09:45:33PM +0100, Juan Quintela wrote: >> It was used as a size in all cases except one. > > Considering that: > > - qemu_target_page_bits() is only used in migration codes, in only > several places below > > - migration codes is using TARGET_PAGE_{BITS|SIZE} a lot as well TARGET_PAGE_* is only defined for target specific files, migration (in general) is not one of them (ram.c on the other hand is). Until we exported that function, there wasn't a way to know the TARGET_PAGE_SIZE in migration.c, for instance. > > How about we just remove this function, and directly use > TARGET_PAGE_{BITS|SIZE}? We can't. This was the reason why we used to have exported the sizes in bytes and in pages. There are files in qemu that are compiled by target, and there are files that are compiled the same for all targets. Later, Juan.
diff --git a/exec.c b/exec.c index e57a8a2..9a4c385 100644 --- a/exec.c +++ b/exec.c @@ -3349,9 +3349,9 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, * Allows code that needs to deal with migration bitmaps etc to still be built * target independent. */ -size_t qemu_target_page_bits(void) +size_t qemu_target_page_size(void) { - return TARGET_PAGE_BITS; + return TARGET_PAGE_SIZE; } #endif diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 576c7ce..16175f7 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -67,7 +67,7 @@ int qemu_reset_requested_get(void); void qemu_system_killed(int signal, pid_t pid); void qemu_system_reset(bool report); void qemu_system_guest_panicked(GuestPanicInformation *info); -size_t qemu_target_page_bits(void); +size_t qemu_target_page_size(void); void qemu_add_exit_notifier(Notifier *notify); void qemu_remove_exit_notifier(Notifier *notify); diff --git a/migration/migration.c b/migration/migration.c index 3f99ab3..92c3c6b 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -646,7 +646,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) info->ram->skipped = 0; info->ram->normal = norm_mig_pages_transferred(); info->ram->normal_bytes = norm_mig_pages_transferred() * - (1ul << qemu_target_page_bits()); + qemu_target_page_size(); info->ram->mbps = s->mbps; info->ram->dirty_sync_count = ram_dirty_sync_count(); info->ram->postcopy_requests = ram_postcopy_requests(); @@ -2001,7 +2001,7 @@ static void *migration_thread(void *opaque) 10000 is a small enough number for our purposes */ if (ram_dirty_pages_rate() && transferred_bytes > 10000) { s->expected_downtime = ram_dirty_pages_rate() * - (1ul << qemu_target_page_bits()) / bandwidth; + qemu_target_page_size() / bandwidth; } qemu_file_reset_rate_limit(s->to_dst_file); diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index dc80dbb..8756364 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -123,7 +123,7 @@ bool postcopy_ram_supported_by_host(void) struct uffdio_range range_struct; uint64_t feature_mask; - if ((1ul << qemu_target_page_bits()) > pagesize) { + if (qemu_target_page_size() > pagesize) { error_report("Target page size bigger than host page size"); goto out; } @@ -745,10 +745,10 @@ PostcopyDiscardState *postcopy_discard_send_init(MigrationState *ms, void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState *pds, unsigned long start, unsigned long length) { - size_t tp_bits = qemu_target_page_bits(); + size_t tp_size = qemu_target_page_size(); /* Convert to byte offsets within the RAM block */ - pds->start_list[pds->cur_entry] = (start - pds->offset) << tp_bits; - pds->length_list[pds->cur_entry] = length << tp_bits; + pds->start_list[pds->cur_entry] = (start - pds->offset) * tp_size; + pds->length_list[pds->cur_entry] = length * tp_size; trace_postcopy_discard_send_range(pds->ramblock_name, start, length); pds->cur_entry++; pds->nsentwords++; diff --git a/migration/savevm.c b/migration/savevm.c index 853a81a..bbf055d 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -871,7 +871,7 @@ void qemu_savevm_send_postcopy_advise(QEMUFile *f) { uint64_t tmp[2]; tmp[0] = cpu_to_be64(ram_pagesize_summary()); - tmp[1] = cpu_to_be64(1ul << qemu_target_page_bits()); + tmp[1] = cpu_to_be64(qemu_target_page_size()); trace_qemu_savevm_send_postcopy_advise(); qemu_savevm_command_send(f, MIG_CMD_POSTCOPY_ADVISE, 16, (uint8_t *)tmp); @@ -1390,13 +1390,13 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis) } remote_tps = qemu_get_be64(mis->from_src_file); - if (remote_tps != (1ul << qemu_target_page_bits())) { + if (remote_tps != qemu_target_page_size()) { /* * Again, some differences could be dealt with, but for now keep it * simple. */ - error_report("Postcopy needs matching target page sizes (s=%d d=%d)", - (int)remote_tps, 1 << qemu_target_page_bits()); + error_report("Postcopy needs matching target page sizes (s=%d d=%zd)", + (int)remote_tps, qemu_target_page_size()); return -1; }
It was used as a size in all cases except one. Signed-off-by: Juan Quintela <quintela@redhat.com> --- exec.c | 4 ++-- include/sysemu/sysemu.h | 2 +- migration/migration.c | 4 ++-- migration/postcopy-ram.c | 8 ++++---- migration/savevm.c | 8 ++++---- 5 files changed, 13 insertions(+), 13 deletions(-)