Re: [PATCH] softmmu: Tidy dirtylimit_dirty_ring_full_time

[Philippe Mathieu-Daudé]

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,