| Message ID | 20230413054509.54421-1-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | softmmu: Move dirtylimit.c into the target independent source set | expand |
On 4/13/23 07:45, Thomas Huth wrote: > uint32_t dirty_ring_size = kvm_dirty_ring_size(); > uint64_t dirty_ring_size_meory_MB = > - dirty_ring_size * TARGET_PAGE_SIZE >> 20; > + dirty_ring_size * qemu_target_page_size() >> 20; Existing problem, the types here are suspicious: dirty_ring_size is uint32_t, dirty_ring_size_meory (typo) is uint64_t. I wonder if this is better computed as uint32_t dirty_ring_size_MB = dirty_ring_size >> (20 - qemu_target_page_bits()); r~
Thomas Huth <thuth@redhat.com> writes: > dirtylimit.c just uses one TARGET_PAGE_SIZE macro - change it to > qemu_target_page_size() so we can move thefile into the target > independent source set. Then we only have to compile this file > once during the build instead of multiple times (one time for > each target). > > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de>
On 13/04/2023 12.31, Richard Henderson wrote: > On 4/13/23 07:45, Thomas Huth wrote: >> uint32_t dirty_ring_size = kvm_dirty_ring_size(); >> uint64_t dirty_ring_size_meory_MB = >> - dirty_ring_size * TARGET_PAGE_SIZE >> 20; >> + dirty_ring_size * qemu_target_page_size() >> 20; > > Existing problem, the types here are suspicious: dirty_ring_size is > uint32_t, dirty_ring_size_meory (typo) is uint64_t. > > I wonder if this is better computed as > > uint32_t dirty_ring_size_MB = dirty_ring_size >> (20 - > qemu_target_page_bits()); qemu_target_page_size() returns a "size_t", so I think it should be fine for 64-bit hosts. But for 32-bit hosts, this looks error prone, indeed, so I think your suggestion is a good idea. Care to send a patch? Thomas
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c index c56f0f58c8..82986c1499 100644 --- a/softmmu/dirtylimit.c +++ b/softmmu/dirtylimit.c @@ -20,6 +20,7 @@ #include "monitor/hmp.h" #include "monitor/monitor.h" #include "exec/memory.h" +#include "exec/target_page.h" #include "hw/boards.h" #include "sysemu/kvm.h" #include "trace.h" @@ -236,7 +237,7 @@ static inline int64_t dirtylimit_dirty_ring_full_time(uint64_t dirtyrate) static uint64_t max_dirtyrate; uint32_t dirty_ring_size = kvm_dirty_ring_size(); uint64_t dirty_ring_size_meory_MB = - dirty_ring_size * TARGET_PAGE_SIZE >> 20; + dirty_ring_size * qemu_target_page_size() >> 20; if (max_dirtyrate < dirtyrate) { max_dirtyrate = dirtyrate; diff --git a/softmmu/meson.build b/softmmu/meson.build index b392f0bd35..974732b0f3 100644 --- a/softmmu/meson.build +++ b/softmmu/meson.build @@ -3,7 +3,6 @@ specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files( 'ioport.c', 'memory.c', 'physmem.c', - 'dirtylimit.c', 'watchpoint.c', )]) @@ -18,6 +17,7 @@ softmmu_ss.add(files( 'cpu-throttle.c', 'cpu-timers.c', 'datadir.c', + 'dirtylimit.c', 'dma-helpers.c', 'globals.c', 'memory_mapping.c',
dirtylimit.c just uses one TARGET_PAGE_SIZE macro - change it to qemu_target_page_size() so we can move thefile into the target independent source set. Then we only have to compile this file once during the build instead of multiple times (one time for each target). Signed-off-by: Thomas Huth <thuth@redhat.com> --- softmmu/dirtylimit.c | 3 ++- softmmu/meson.build | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)