Possible memory release problem

Possible memory release problem

[Михаил Северов]

[-- Attachment #1: Type: text/plain, Size: 193 bytes --]

Colleagues, can you tell me in /migration/ram.c :2837 whether it is not
necessary to perform memory release for block->bmap, after memory has been
allocated in :2831. If not please explain why

[-- Attachment #2: Type: text/html, Size: 221 bytes --]

Re: Possible memory release problem

[Alex Bennée]

Михаил Северов <senserk71@gmail.com> writes:

> Colleagues, can you tell me in /migration/ram.c :2837 whether it is not necessary to perform memory release for
> block->bmap, after memory has been allocated in :2831. If not please
> explain why

It's easier if you post the context for the code as things do move
around:

  static void ram_list_init_bitmaps(void)
  {
      MigrationState *ms = migrate_get_current();
      RAMBlock *block;
      unsigned long pages;
      uint8_t shift;

      /* Skip setting bitmap if there is no RAM */
      if (ram_bytes_total()) {
          shift = ms->clear_bitmap_shift;
          if (shift > CLEAR_BITMAP_SHIFT_MAX) {
              error_report("clear_bitmap_shift (%u) too big, using "
                           "max value (%u)", shift, CLEAR_BITMAP_SHIFT_MAX);
              shift = CLEAR_BITMAP_SHIFT_MAX;
          } else if (shift < CLEAR_BITMAP_SHIFT_MIN) {
              error_report("clear_bitmap_shift (%u) too small, using "
                           "min value (%u)", shift, CLEAR_BITMAP_SHIFT_MIN);
              shift = CLEAR_BITMAP_SHIFT_MIN;
          }

          RAMBLOCK_FOREACH_NOT_IGNORED(block) {
              pages = block->max_length >> TARGET_PAGE_BITS;
              /*
               * The initial dirty bitmap for migration must be set with all
               * ones to make sure we'll migrate every guest RAM page to
               * destination.
               * Here we set RAMBlock.bmap all to 1 because when rebegin a
               * new migration after a failed migration, ram_list.
               * dirty_memory[DIRTY_MEMORY_MIGRATION] don't include the whole
               * guest memory.
               */
              block->bmap = bitmap_new(pages);
              bitmap_set(block->bmap, 0, pages);
              if (migrate_mapped_ram()) {
                  block->file_bmap = bitmap_new(pages);
              }
              block->clear_bmap_shift = shift;
              block->clear_bmap = bitmap_new(clear_bmap_size(pages, shift));
          }
      }
  }

On my copy the code the two bitmap_new()'s are for different variables.
Are you asking where they are freed? AFAICT here:

  static void ram_save_cleanup(void *opaque)
  {
      RAMState **rsp = opaque;
      RAMBlock *block;

      /* We don't use dirty log with background snapshots */
      if (!migrate_background_snapshot()) {
          /* caller have hold BQL or is in a bh, so there is
           * no writing race against the migration bitmap
           */
          if (global_dirty_tracking & GLOBAL_DIRTY_MIGRATION) {
              /*
               * do not stop dirty log without starting it, since
               * memory_global_dirty_log_stop will assert that
               * memory_global_dirty_log_start/stop used in pairs
               */
              memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
          }
      }

      RAMBLOCK_FOREACH_NOT_IGNORED(block) {
          g_free(block->clear_bmap);
          block->clear_bmap = NULL;
          g_free(block->bmap);
          block->bmap = NULL;
      }

      xbzrle_cleanup();
      compress_threads_save_cleanup();
      ram_state_cleanup(rsp);
      g_free(migration_ops);
      migration_ops = NULL;
  }


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro