Re: [PATCH RFC 10/15] migration: Move static var in ram_block_from_stream() into global

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Peter Xu (peterx@redhat.com) wrote:
> Static variable is very unfriendly to threading of ram_block_from_stream().
> Move it into MigrationIncomingState.
> 
> Make the incoming state pointer to be passed over to ram_block_from_stream() on
> both caller sites.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

OK, but I'm not sure if I noticed where you changed this to be per
channel later?



Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Dave
> ---
>  migration/migration.h |  3 ++-
>  migration/ram.c       | 13 +++++++++----
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/migration.h b/migration/migration.h
> index 35e7f7babe..34b79cb961 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -66,7 +66,8 @@ typedef struct {
>  /* State for the incoming migration */
>  struct MigrationIncomingState {
>      QEMUFile *from_src_file;
> -
> +    /* Previously received RAM's RAMBlock pointer */
> +    RAMBlock *last_recv_block;
>      /* A hook to allow cleanup at the end of incoming migration */
>      void *transport_data;
>      void (*transport_cleanup)(void *data);
> diff --git a/migration/ram.c b/migration/ram.c
> index 3f823ffffc..3a7d943f9c 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3183,12 +3183,14 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
>   *
>   * Returns a pointer from within the RCU-protected ram_list.
>   *
> + * @mis: the migration incoming state pointer
>   * @f: QEMUFile where to read the data from
>   * @flags: Page flags (mostly to see if it's a continuation of previous block)
>   */
> -static inline RAMBlock *ram_block_from_stream(QEMUFile *f, int flags)
> +static inline RAMBlock *ram_block_from_stream(MigrationIncomingState *mis,
> +                                              QEMUFile *f, int flags)
>  {
> -    static RAMBlock *block;
> +    RAMBlock *block = mis->last_recv_block;
>      char id[256];
>      uint8_t len;
>  
> @@ -3215,6 +3217,8 @@ static inline RAMBlock *ram_block_from_stream(QEMUFile *f, int flags)
>          return NULL;
>      }
>  
> +    mis->last_recv_block = block;
> +
>      return block;
>  }
>  
> @@ -3667,7 +3671,7 @@ static int ram_load_postcopy(QEMUFile *f)
>          trace_ram_load_postcopy_loop((uint64_t)addr, flags);
>          if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
>                       RAM_SAVE_FLAG_COMPRESS_PAGE)) {
> -            block = ram_block_from_stream(f, flags);
> +            block = ram_block_from_stream(mis, f, flags);
>              if (!block) {
>                  ret = -EINVAL;
>                  break;
> @@ -3881,6 +3885,7 @@ void colo_flush_ram_cache(void)
>   */
>  static int ram_load_precopy(QEMUFile *f)
>  {
> +    MigrationIncomingState *mis = migration_incoming_get_current();
>      int flags = 0, ret = 0, invalid_flags = 0, len = 0, i = 0;
>      /* ADVISE is earlier, it shows the source has the postcopy capability on */
>      bool postcopy_advised = postcopy_is_advised();
> @@ -3919,7 +3924,7 @@ static int ram_load_precopy(QEMUFile *f)
>  
>          if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
>                       RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
> -            RAMBlock *block = ram_block_from_stream(f, flags);
> +            RAMBlock *block = ram_block_from_stream(mis, f, flags);
>  
>              host = host_from_ram_block_offset(block, addr);
>              /*
> -- 
> 2.32.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK