Re: [PATCH 12/18] migration: Create compression_update_rates()

[Lukas Straub]

On Tue, 13 Jun 2023 16:57:51 +0200
Juan Quintela <quintela@redhat.com> wrote:

> So we can move more compression_counters stuff to ram-compress.c.
> Create compression_counters struct to add the stuff that was on
> MigrationState.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

With the comment below fixed:
Reviewed-by: Lukas Straub <lukasstraub2@web.de>

> ---
>  migration/ram-compress.h |  1 +
>  migration/ram.h          |  1 -
>  migration/ram-compress.c | 41 +++++++++++++++++++++++++++++++++++++++-
>  migration/ram.c          | 29 +---------------------------
>  4 files changed, 42 insertions(+), 30 deletions(-)
> 
> diff --git a/migration/ram-compress.h b/migration/ram-compress.h
> index b228640092..a7eb028849 100644
> --- a/migration/ram-compress.h
> +++ b/migration/ram-compress.h
> @@ -71,5 +71,6 @@ void decompress_data_with_multi_threads(QEMUFile *f, void 
> *host, int len);
>  void populate_compress(MigrationInfo *info);
>  uint64_t ram_compressed_pages(void);
>  void update_compress_thread_counts(const CompressParam *param, int 
> bytes_xmit);
> +void compression_update_rates(uint64_t page_count);
>  
>  #endif
> diff --git a/migration/ram.h b/migration/ram.h
> index ea1f3c25b5..60bc4c9e3a 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -34,7 +34,6 @@
>  #include "io/channel.h"
>  
>  extern XBZRLECacheStats xbzrle_counters;
> -extern CompressionStats compression_counters;
>  
>  bool ramblock_is_ignored(RAMBlock *block);
>  /* Should be holding either ram_list.mutex, or the RCU lock. */
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index 2652cdee8b..5e1bb82fcd 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -41,7 +41,20 @@
>  #include "ram.h"
>  #include "migration-stats.h"
>  
> -CompressionStats compression_counters;
> +static struct {
> +    int64_t pages;
> +    int64_t busy;
> +    double busy_rate;
> +    int64_t compressed_size;
> +    double compression_rate;
> +    /* compression statistics since the beginning of the period */
> +    /* amount of count that no free thread to compress data */
> +    uint64_t compress_thread_busy_prev;
> +    /* amount bytes after compression */
> +    uint64_t compressed_size_prev;
> +    /* amount of compressed pages */
> +    uint64_t compress_pages_prev;
> +} compression_counters;
>  
>  static CompressParam *comp_param;
>  static QemuThread *compress_threads;
> @@ -529,3 +542,29 @@ void update_compress_thread_counts(const CompressParam 
> *param, int bytes_xmit)
>      compression_counters.pages++;
>  }
>  
> +void compression_update_rates(uint64_t page_count)
> +{
> +    if (migrate_compress()) {

Use early return.

> +        compression_counters.busy_rate = (double)(compression_counters.busy -
> +            compression_counters.compress_thread_busy_prev) / page_count;
> +        compression_counters.compress_thread_busy_prev =
> +            compression_counters.busy;
> +
> +        double compressed_size = compression_counters.compressed_size -
> +                                 compression_counters.compressed_size_prev;
> +        if (compressed_size) {
> +            double uncompressed_size = (compression_counters.pages -
> +                compression_counters.compress_pages_prev) *
> +                qemu_target_page_size();
> +
> +            /* Compression-Ratio = Uncompressed-size / Compressed-size */
> +            compression_counters.compression_rate =
> +                                        uncompressed_size / compressed_size;
> +
> +            compression_counters.compress_pages_prev =
> +                compression_counters.pages;
> +            compression_counters.compressed_size_prev =
> +                compression_counters.compressed_size;
> +        }
> +    }
> +}
> diff --git a/migration/ram.c b/migration/ram.c
> index 60f24006bc..1bd586c23a 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -364,13 +364,6 @@ struct RAMState {
>      bool xbzrle_started;
>      /* Are we on the last stage of migration */
>      bool last_stage;
> -    /* compression statistics since the beginning of the period */
> -    /* amount of count that no free thread to compress data */
> -    uint64_t compress_thread_busy_prev;
> -    /* amount bytes after compression */
> -    uint64_t compressed_size_prev;
> -    /* amount of compressed pages */
> -    uint64_t compress_pages_prev;
>  
>      /* total handled target pages at the beginning of period */
>      uint64_t target_page_count_prev;
> @@ -933,7 +926,6 @@ uint64_t ram_get_total_transferred_pages(void)
>  static void migration_update_rates(RAMState *rs, int64_t end_time)
>  {
>      uint64_t page_count = rs->target_page_count - rs->target_page_count_prev;
> -    double compressed_size;
>  
>      /* calculate period counters */
>      stat64_set(&mig_stats.dirty_pages_rate,
> @@ -961,26 +953,7 @@ static void migration_update_rates(RAMState *rs, int64_t 
> end_time)
>          rs->xbzrle_pages_prev = xbzrle_counters.pages;
>          rs->xbzrle_bytes_prev = xbzrle_counters.bytes;
>      }
> -
> -    if (migrate_compress()) {
> -        compression_counters.busy_rate = (double)(compression_counters.busy -
> -            rs->compress_thread_busy_prev) / page_count;
> -        rs->compress_thread_busy_prev = compression_counters.busy;
> -
> -        compressed_size = compression_counters.compressed_size -
> -                          rs->compressed_size_prev;
> -        if (compressed_size) {
> -            double uncompressed_size = (compression_counters.pages -
> -                                    rs->compress_pages_prev) * 
> TARGET_PAGE_SIZE;
> -
> -            /* Compression-Ratio = Uncompressed-size / Compressed-size */
> -            compression_counters.compression_rate =
> -                                        uncompressed_size / compressed_size;
> -
> -            rs->compress_pages_prev = compression_counters.pages;
> -            rs->compressed_size_prev = compression_counters.compressed_size;
> -        }
> -    }
> +    compression_update_rates(page_count);
>  }
>  
>  static void migration_trigger_throttle(RAMState *rs)

pgp71EwL6vsmQ.pgp
Description: OpenPGP digital signature