| Message ID | 20230428103448.3811208-1-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | softmmu: Tidy dirtylimit_dirty_ring_full_time | expand |
On 28/04/2023 12.34, Richard Henderson wrote: > Drop inline marker: let compiler decide. > > Change return type to uint64_t: this matches the computation in the > return statement and the local variable assignment in the caller. > > Rename local to dirty_ring_size_MB to fix typo. > Simplify conversion to MiB via qemu_target_page_bits and right shift. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > softmmu/dirtylimit.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c > index 82986c1499..71bf6dc7a4 100644 > --- a/softmmu/dirtylimit.c > +++ b/softmmu/dirtylimit.c > @@ -232,18 +232,23 @@ bool dirtylimit_vcpu_index_valid(int cpu_index) > cpu_index >= ms->smp.max_cpus); > } > > -static inline int64_t dirtylimit_dirty_ring_full_time(uint64_t dirtyrate) > +static uint64_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 * qemu_target_page_size() >> 20; > + unsigned target_page_bits = qemu_target_page_bits(); > + uint64_t dirty_ring_size_MB; > + > + /* So far, the largest (non-huge) page size is 64k, i.e. 16 bits. */ > + assert(target_page_bits < 20); > + > + /* Convert ring size (pages) to MiB (2**20). */ > + dirty_ring_size_MB = kvm_dirty_ring_size() >> (20 - target_page_bits); > > if (max_dirtyrate < dirtyrate) { > max_dirtyrate = dirtyrate; > } > > - return dirty_ring_size_meory_MB * 1000000 / max_dirtyrate; > + return dirty_ring_size_MB * 1000000 / max_dirtyrate; > } Reviewed-by: Thomas Huth <thuth@redhat.com>
Thomas Huth <thuth@redhat.com> wrote: > On 28/04/2023 12.34, Richard Henderson wrote: >> Drop inline marker: let compiler decide. >> Change return type to uint64_t: this matches the computation in the >> return statement and the local variable assignment in the caller. >> Rename local to dirty_ring_size_MB to fix typo. >> Simplify conversion to MiB via qemu_target_page_bits and right shift. >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Juan Quintela <quintela@redhat.com> Not that I expect the amount of bytes in one machine to not fit in 64bit, but good change. About the signesdness of variables, I really hate it, specially that compiler not help here where arguments, operands and assignment place is unsigned. Sniff.
On 4/28/23 19:38, Juan Quintela wrote: > Thomas Huth <thuth@redhat.com> wrote: >> On 28/04/2023 12.34, Richard Henderson wrote: >>> Drop inline marker: let compiler decide. >>> Change return type to uint64_t: this matches the computation in the >>> return statement and the local variable assignment in the caller. >>> Rename local to dirty_ring_size_MB to fix typo. >>> Simplify conversion to MiB via qemu_target_page_bits and right shift. >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > Reviewed-by: Juan Quintela <quintela@redhat.com> Thanks, Queueing to tcg-next. r~
On 28/4/23 12:34, Richard Henderson wrote: > Drop inline marker: let compiler decide. > > Change return type to uint64_t: this matches the computation in the > return statement and the local variable assignment in the caller. > > Rename local to dirty_ring_size_MB to fix typo. > Simplify conversion to MiB via qemu_target_page_bits and right shift. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > softmmu/dirtylimit.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > -static inline int64_t dirtylimit_dirty_ring_full_time(uint64_t dirtyrate) > +static uint64_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 * qemu_target_page_size() >> 20; > + unsigned target_page_bits = qemu_target_page_bits(); > + uint64_t dirty_ring_size_MB; > + > + /* So far, the largest (non-huge) page size is 64k, i.e. 16 bits. */ > + assert(target_page_bits < 20); > + > + /* Convert ring size (pages) to MiB (2**20). */ > + dirty_ring_size_MB = kvm_dirty_ring_size() >> (20 - target_page_bits); > > if (max_dirtyrate < dirtyrate) { > max_dirtyrate = dirtyrate; > } > > - return dirty_ring_size_meory_MB * 1000000 / max_dirtyrate; > + return dirty_ring_size_MB * 1000000 / max_dirtyrate; I wondered what was this 1000000 magic number. IIUC this is converted to microseconds. We don't have such definition. We could use (NANOSECONDS_PER_SECOND / 1000). Somehow simpler is to rename dirtylimit_dirty_ring_full_time() -> dirtylimit_dirty_ring_full_time_us(). Anyway, Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > } > > static inline bool dirtylimit_done(uint64_t quota,
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c index 82986c1499..71bf6dc7a4 100644 --- a/softmmu/dirtylimit.c +++ b/softmmu/dirtylimit.c @@ -232,18 +232,23 @@ bool dirtylimit_vcpu_index_valid(int cpu_index) cpu_index >= ms->smp.max_cpus); } -static inline int64_t dirtylimit_dirty_ring_full_time(uint64_t dirtyrate) +static uint64_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 * qemu_target_page_size() >> 20; + unsigned target_page_bits = qemu_target_page_bits(); + uint64_t dirty_ring_size_MB; + + /* So far, the largest (non-huge) page size is 64k, i.e. 16 bits. */ + assert(target_page_bits < 20); + + /* Convert ring size (pages) to MiB (2**20). */ + dirty_ring_size_MB = kvm_dirty_ring_size() >> (20 - target_page_bits); if (max_dirtyrate < dirtyrate) { max_dirtyrate = dirtyrate; } - return dirty_ring_size_meory_MB * 1000000 / max_dirtyrate; + return dirty_ring_size_MB * 1000000 / max_dirtyrate; } static inline bool dirtylimit_done(uint64_t quota,
Drop inline marker: let compiler decide. Change return type to uint64_t: this matches the computation in the return statement and the local variable assignment in the caller. Rename local to dirty_ring_size_MB to fix typo. Simplify conversion to MiB via qemu_target_page_bits and right shift. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- softmmu/dirtylimit.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)