| Message ID | 20170315135021.6978-24-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
* Juan Quintela (quintela@redhat.com) wrote: > Once there, rename the type to be shorter. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > migration/ram.c | 79 ++++++++++++++++++++++++++++++--------------------------- > 1 file changed, 42 insertions(+), 37 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index c14293c..d39d185 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -132,6 +132,19 @@ out: > return ret; > } > > +struct RAMBitmap { > + struct rcu_head rcu; > + /* Main migration bitmap */ > + unsigned long *bmap; > + /* bitmap of pages that haven't been sent even once > + * only maintained and used in postcopy at the moment > + * where it's used to send the dirtymap at the start > + * of the postcopy phase > + */ > + unsigned long *unsentmap; > +}; > +typedef struct RAMBitmap RAMBitmap; > + I'm OK with this; although I can see the idea of naming it BitmapRcu, given that the actual bmap is inside that and most of the rest of the type is just the rcu wrapper. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > /* State of RAM for migration */ > struct RAMState { > /* Last block that we have visited searching for dirty pages */ > @@ -180,6 +193,8 @@ struct RAMState { > uint64_t migration_dirty_pages; > /* protects modification of the bitmap */ > QemuMutex bitmap_mutex; > + /* Ram Bitmap protected by RCU */ > + RAMBitmap *ram_bitmap; > }; > typedef struct RAMState RAMState; > > @@ -236,18 +251,6 @@ struct PageSearchStatus { > }; > typedef struct PageSearchStatus PageSearchStatus; > > -static struct BitmapRcu { > - struct rcu_head rcu; > - /* Main migration bitmap */ > - unsigned long *bmap; > - /* bitmap of pages that haven't been sent even once > - * only maintained and used in postcopy at the moment > - * where it's used to send the dirtymap at the start > - * of the postcopy phase > - */ > - unsigned long *unsentmap; > -} *migration_bitmap_rcu; > - > struct CompressParam { > bool done; > bool quit; > @@ -554,7 +557,7 @@ ram_addr_t migration_bitmap_find_dirty(RAMState *rs, RAMBlock *rb, > > unsigned long next; > > - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; > + bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap; > if (rs->ram_bulk_stage && nr > base) { > next = nr + 1; > } else { > @@ -569,7 +572,7 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs, ram_addr_t addr) > { > bool ret; > int nr = addr >> TARGET_PAGE_BITS; > - unsigned long *bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; > + unsigned long *bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap; > > ret = test_and_clear_bit(nr, bitmap); > > @@ -583,7 +586,7 @@ static void migration_bitmap_sync_range(RAMState *rs, ram_addr_t start, > ram_addr_t length) > { > unsigned long *bitmap; > - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; > + bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap; > rs-> migration_dirty_pages += > cpu_physical_memory_sync_dirty_bitmap(bitmap, start, length); > } > @@ -1115,14 +1118,14 @@ static bool get_queued_page(RAMState *rs, MigrationState *ms, PageSearchStatus * > */ > if (block) { > unsigned long *bitmap; > - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; > + bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap; > dirty = test_bit(*ram_addr_abs >> TARGET_PAGE_BITS, bitmap); > if (!dirty) { > trace_get_queued_page_not_dirty( > block->idstr, (uint64_t)offset, > (uint64_t)*ram_addr_abs, > test_bit(*ram_addr_abs >> TARGET_PAGE_BITS, > - atomic_rcu_read(&migration_bitmap_rcu)->unsentmap)); > + atomic_rcu_read(&rs->ram_bitmap)->unsentmap)); > } else { > trace_get_queued_page(block->idstr, > (uint64_t)offset, > @@ -1276,7 +1279,7 @@ static int ram_save_target_page(RAMState *rs, MigrationState *ms, QEMUFile *f, > if (res < 0) { > return res; > } > - unsentmap = atomic_rcu_read(&migration_bitmap_rcu)->unsentmap; > + unsentmap = atomic_rcu_read(&rs->ram_bitmap)->unsentmap; > if (unsentmap) { > clear_bit(dirty_ram_abs >> TARGET_PAGE_BITS, unsentmap); > } > @@ -1440,7 +1443,7 @@ void free_xbzrle_decoded_buf(void) > xbzrle_decoded_buf = NULL; > } > > -static void migration_bitmap_free(struct BitmapRcu *bmap) > +static void migration_bitmap_free(struct RAMBitmap *bmap) > { > g_free(bmap->bmap); > g_free(bmap->unsentmap); > @@ -1449,11 +1452,13 @@ static void migration_bitmap_free(struct BitmapRcu *bmap) > > static void ram_migration_cleanup(void *opaque) > { > + RAMState *rs = opaque; > + > /* caller have hold iothread lock or is in a bh, so there is > * no writing race against this migration_bitmap > */ > - struct BitmapRcu *bitmap = migration_bitmap_rcu; > - atomic_rcu_set(&migration_bitmap_rcu, NULL); > + struct RAMBitmap *bitmap = rs->ram_bitmap; > + atomic_rcu_set(&rs->ram_bitmap, NULL); > if (bitmap) { > memory_global_dirty_log_stop(); > call_rcu(bitmap, migration_bitmap_free, rcu); > @@ -1488,9 +1493,9 @@ void migration_bitmap_extend(ram_addr_t old, ram_addr_t new) > /* called in qemu main thread, so there is > * no writing race against this migration_bitmap > */ > - if (migration_bitmap_rcu) { > - struct BitmapRcu *old_bitmap = migration_bitmap_rcu, *bitmap; > - bitmap = g_new(struct BitmapRcu, 1); > + if (ram_state.ram_bitmap) { > + struct RAMBitmap *old_bitmap = ram_state.ram_bitmap, *bitmap; > + bitmap = g_new(struct RAMBitmap, 1); > bitmap->bmap = bitmap_new(new); > > /* prevent migration_bitmap content from being set bit > @@ -1508,7 +1513,7 @@ void migration_bitmap_extend(ram_addr_t old, ram_addr_t new) > */ > bitmap->unsentmap = NULL; > > - atomic_rcu_set(&migration_bitmap_rcu, bitmap); > + atomic_rcu_set(&ram_state.ram_bitmap, bitmap); > qemu_mutex_unlock(&ram_state.bitmap_mutex); > ram_state.migration_dirty_pages += new - old; > call_rcu(old_bitmap, migration_bitmap_free, rcu); > @@ -1529,7 +1534,7 @@ void ram_debug_dump_bitmap(unsigned long *todump, bool expected) > char linebuf[129]; > > if (!todump) { > - todump = atomic_rcu_read(&migration_bitmap_rcu)->bmap; > + todump = atomic_rcu_read(&ram_state.ram_bitmap)->bmap; > } > > for (cur = 0; cur < ram_pages; cur += linelen) { > @@ -1559,7 +1564,7 @@ void ram_debug_dump_bitmap(unsigned long *todump, bool expected) > void ram_postcopy_migrated_memory_release(MigrationState *ms) > { > struct RAMBlock *block; > - unsigned long *bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; > + unsigned long *bitmap = atomic_rcu_read(&ram_state.ram_bitmap)->bmap; > > QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > unsigned long first = block->offset >> TARGET_PAGE_BITS; > @@ -1591,7 +1596,7 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms, > unsigned long current; > unsigned long *unsentmap; > > - unsentmap = atomic_rcu_read(&migration_bitmap_rcu)->unsentmap; > + unsentmap = atomic_rcu_read(&ram_state.ram_bitmap)->unsentmap; > for (current = start; current < end; ) { > unsigned long one = find_next_bit(unsentmap, end, current); > > @@ -1680,8 +1685,8 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, > return; > } > > - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; > - unsentmap = atomic_rcu_read(&migration_bitmap_rcu)->unsentmap; > + bitmap = atomic_rcu_read(&ram_state.ram_bitmap)->bmap; > + unsentmap = atomic_rcu_read(&ram_state.ram_bitmap)->unsentmap; > > if (unsent_pass) { > /* Find a sent page */ > @@ -1836,7 +1841,7 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms) > /* This should be our last sync, the src is now paused */ > migration_bitmap_sync(&ram_state); > > - unsentmap = atomic_rcu_read(&migration_bitmap_rcu)->unsentmap; > + unsentmap = atomic_rcu_read(&ram_state.ram_bitmap)->unsentmap; > if (!unsentmap) { > /* We don't have a safe way to resize the sentmap, so > * if the bitmap was resized it will be NULL at this > @@ -1857,7 +1862,7 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms) > /* > * Update the unsentmap to be unsentmap = unsentmap | dirty > */ > - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; > + bitmap = atomic_rcu_read(&ram_state.ram_bitmap)->bmap; > bitmap_or(unsentmap, unsentmap, bitmap, > last_ram_offset() >> TARGET_PAGE_BITS); > > @@ -1950,16 +1955,16 @@ static int ram_state_init(RAMState *rs) > bytes_transferred = 0; > ram_state_reset(rs); > > - migration_bitmap_rcu = g_new0(struct BitmapRcu, 1); > + rs->ram_bitmap = g_new0(struct RAMBitmap, 1); > /* Skip setting bitmap if there is no RAM */ > if (ram_bytes_total()) { > ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS; > - migration_bitmap_rcu->bmap = bitmap_new(ram_bitmap_pages); > - bitmap_set(migration_bitmap_rcu->bmap, 0, ram_bitmap_pages); > + rs->ram_bitmap->bmap = bitmap_new(ram_bitmap_pages); > + bitmap_set(rs->ram_bitmap->bmap, 0, ram_bitmap_pages); > > if (migrate_postcopy_ram()) { > - migration_bitmap_rcu->unsentmap = bitmap_new(ram_bitmap_pages); > - bitmap_set(migration_bitmap_rcu->unsentmap, 0, ram_bitmap_pages); > + rs->ram_bitmap->unsentmap = bitmap_new(ram_bitmap_pages); > + bitmap_set(rs->ram_bitmap->unsentmap, 0, ram_bitmap_pages); > } > } > > -- > 2.9.3 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> Once there, rename the type to be shorter. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> migration/ram.c | 79 ++++++++++++++++++++++++++++++--------------------------- >> 1 file changed, 42 insertions(+), 37 deletions(-) >> >> diff --git a/migration/ram.c b/migration/ram.c >> index c14293c..d39d185 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -132,6 +132,19 @@ out: >> return ret; >> } >> >> +struct RAMBitmap { >> + struct rcu_head rcu; >> + /* Main migration bitmap */ >> + unsigned long *bmap; >> + /* bitmap of pages that haven't been sent even once >> + * only maintained and used in postcopy at the moment >> + * where it's used to send the dirtymap at the start >> + * of the postcopy phase >> + */ >> + unsigned long *unsentmap; >> +}; >> +typedef struct RAMBitmap RAMBitmap; >> + > > I'm OK with this; although I can see the idea of naming it BitmapRcu, > given that the actual bmap is inside that and most of the rest of the type > is just the rcu wrapper. It is the type, and now it also has the unsentmap. atomic_rcu_read(&ram_state.bitmap_rcu)->bmap) ends getting really long quite fast "p" > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Thanks, Juan.
diff --git a/migration/ram.c b/migration/ram.c index c14293c..d39d185 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -132,6 +132,19 @@ out: return ret; } +struct RAMBitmap { + struct rcu_head rcu; + /* Main migration bitmap */ + unsigned long *bmap; + /* bitmap of pages that haven't been sent even once + * only maintained and used in postcopy at the moment + * where it's used to send the dirtymap at the start + * of the postcopy phase + */ + unsigned long *unsentmap; +}; +typedef struct RAMBitmap RAMBitmap; + /* State of RAM for migration */ struct RAMState { /* Last block that we have visited searching for dirty pages */ @@ -180,6 +193,8 @@ struct RAMState { uint64_t migration_dirty_pages; /* protects modification of the bitmap */ QemuMutex bitmap_mutex; + /* Ram Bitmap protected by RCU */ + RAMBitmap *ram_bitmap; }; typedef struct RAMState RAMState; @@ -236,18 +251,6 @@ struct PageSearchStatus { }; typedef struct PageSearchStatus PageSearchStatus; -static struct BitmapRcu { - struct rcu_head rcu; - /* Main migration bitmap */ - unsigned long *bmap; - /* bitmap of pages that haven't been sent even once - * only maintained and used in postcopy at the moment - * where it's used to send the dirtymap at the start - * of the postcopy phase - */ - unsigned long *unsentmap; -} *migration_bitmap_rcu; - struct CompressParam { bool done; bool quit; @@ -554,7 +557,7 @@ ram_addr_t migration_bitmap_find_dirty(RAMState *rs, RAMBlock *rb, unsigned long next; - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; + bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap; if (rs->ram_bulk_stage && nr > base) { next = nr + 1; } else { @@ -569,7 +572,7 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs, ram_addr_t addr) { bool ret; int nr = addr >> TARGET_PAGE_BITS; - unsigned long *bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; + unsigned long *bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap; ret = test_and_clear_bit(nr, bitmap); @@ -583,7 +586,7 @@ static void migration_bitmap_sync_range(RAMState *rs, ram_addr_t start, ram_addr_t length) { unsigned long *bitmap; - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; + bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap; rs-> migration_dirty_pages += cpu_physical_memory_sync_dirty_bitmap(bitmap, start, length); } @@ -1115,14 +1118,14 @@ static bool get_queued_page(RAMState *rs, MigrationState *ms, PageSearchStatus * */ if (block) { unsigned long *bitmap; - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; + bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap; dirty = test_bit(*ram_addr_abs >> TARGET_PAGE_BITS, bitmap); if (!dirty) { trace_get_queued_page_not_dirty( block->idstr, (uint64_t)offset, (uint64_t)*ram_addr_abs, test_bit(*ram_addr_abs >> TARGET_PAGE_BITS, - atomic_rcu_read(&migration_bitmap_rcu)->unsentmap)); + atomic_rcu_read(&rs->ram_bitmap)->unsentmap)); } else { trace_get_queued_page(block->idstr, (uint64_t)offset, @@ -1276,7 +1279,7 @@ static int ram_save_target_page(RAMState *rs, MigrationState *ms, QEMUFile *f, if (res < 0) { return res; } - unsentmap = atomic_rcu_read(&migration_bitmap_rcu)->unsentmap; + unsentmap = atomic_rcu_read(&rs->ram_bitmap)->unsentmap; if (unsentmap) { clear_bit(dirty_ram_abs >> TARGET_PAGE_BITS, unsentmap); } @@ -1440,7 +1443,7 @@ void free_xbzrle_decoded_buf(void) xbzrle_decoded_buf = NULL; } -static void migration_bitmap_free(struct BitmapRcu *bmap) +static void migration_bitmap_free(struct RAMBitmap *bmap) { g_free(bmap->bmap); g_free(bmap->unsentmap); @@ -1449,11 +1452,13 @@ static void migration_bitmap_free(struct BitmapRcu *bmap) static void ram_migration_cleanup(void *opaque) { + RAMState *rs = opaque; + /* caller have hold iothread lock or is in a bh, so there is * no writing race against this migration_bitmap */ - struct BitmapRcu *bitmap = migration_bitmap_rcu; - atomic_rcu_set(&migration_bitmap_rcu, NULL); + struct RAMBitmap *bitmap = rs->ram_bitmap; + atomic_rcu_set(&rs->ram_bitmap, NULL); if (bitmap) { memory_global_dirty_log_stop(); call_rcu(bitmap, migration_bitmap_free, rcu); @@ -1488,9 +1493,9 @@ void migration_bitmap_extend(ram_addr_t old, ram_addr_t new) /* called in qemu main thread, so there is * no writing race against this migration_bitmap */ - if (migration_bitmap_rcu) { - struct BitmapRcu *old_bitmap = migration_bitmap_rcu, *bitmap; - bitmap = g_new(struct BitmapRcu, 1); + if (ram_state.ram_bitmap) { + struct RAMBitmap *old_bitmap = ram_state.ram_bitmap, *bitmap; + bitmap = g_new(struct RAMBitmap, 1); bitmap->bmap = bitmap_new(new); /* prevent migration_bitmap content from being set bit @@ -1508,7 +1513,7 @@ void migration_bitmap_extend(ram_addr_t old, ram_addr_t new) */ bitmap->unsentmap = NULL; - atomic_rcu_set(&migration_bitmap_rcu, bitmap); + atomic_rcu_set(&ram_state.ram_bitmap, bitmap); qemu_mutex_unlock(&ram_state.bitmap_mutex); ram_state.migration_dirty_pages += new - old; call_rcu(old_bitmap, migration_bitmap_free, rcu); @@ -1529,7 +1534,7 @@ void ram_debug_dump_bitmap(unsigned long *todump, bool expected) char linebuf[129]; if (!todump) { - todump = atomic_rcu_read(&migration_bitmap_rcu)->bmap; + todump = atomic_rcu_read(&ram_state.ram_bitmap)->bmap; } for (cur = 0; cur < ram_pages; cur += linelen) { @@ -1559,7 +1564,7 @@ void ram_debug_dump_bitmap(unsigned long *todump, bool expected) void ram_postcopy_migrated_memory_release(MigrationState *ms) { struct RAMBlock *block; - unsigned long *bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; + unsigned long *bitmap = atomic_rcu_read(&ram_state.ram_bitmap)->bmap; QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { unsigned long first = block->offset >> TARGET_PAGE_BITS; @@ -1591,7 +1596,7 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms, unsigned long current; unsigned long *unsentmap; - unsentmap = atomic_rcu_read(&migration_bitmap_rcu)->unsentmap; + unsentmap = atomic_rcu_read(&ram_state.ram_bitmap)->unsentmap; for (current = start; current < end; ) { unsigned long one = find_next_bit(unsentmap, end, current); @@ -1680,8 +1685,8 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, return; } - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; - unsentmap = atomic_rcu_read(&migration_bitmap_rcu)->unsentmap; + bitmap = atomic_rcu_read(&ram_state.ram_bitmap)->bmap; + unsentmap = atomic_rcu_read(&ram_state.ram_bitmap)->unsentmap; if (unsent_pass) { /* Find a sent page */ @@ -1836,7 +1841,7 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms) /* This should be our last sync, the src is now paused */ migration_bitmap_sync(&ram_state); - unsentmap = atomic_rcu_read(&migration_bitmap_rcu)->unsentmap; + unsentmap = atomic_rcu_read(&ram_state.ram_bitmap)->unsentmap; if (!unsentmap) { /* We don't have a safe way to resize the sentmap, so * if the bitmap was resized it will be NULL at this @@ -1857,7 +1862,7 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms) /* * Update the unsentmap to be unsentmap = unsentmap | dirty */ - bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; + bitmap = atomic_rcu_read(&ram_state.ram_bitmap)->bmap; bitmap_or(unsentmap, unsentmap, bitmap, last_ram_offset() >> TARGET_PAGE_BITS); @@ -1950,16 +1955,16 @@ static int ram_state_init(RAMState *rs) bytes_transferred = 0; ram_state_reset(rs); - migration_bitmap_rcu = g_new0(struct BitmapRcu, 1); + rs->ram_bitmap = g_new0(struct RAMBitmap, 1); /* Skip setting bitmap if there is no RAM */ if (ram_bytes_total()) { ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS; - migration_bitmap_rcu->bmap = bitmap_new(ram_bitmap_pages); - bitmap_set(migration_bitmap_rcu->bmap, 0, ram_bitmap_pages); + rs->ram_bitmap->bmap = bitmap_new(ram_bitmap_pages); + bitmap_set(rs->ram_bitmap->bmap, 0, ram_bitmap_pages); if (migrate_postcopy_ram()) { - migration_bitmap_rcu->unsentmap = bitmap_new(ram_bitmap_pages); - bitmap_set(migration_bitmap_rcu->unsentmap, 0, ram_bitmap_pages); + rs->ram_bitmap->unsentmap = bitmap_new(ram_bitmap_pages); + bitmap_set(rs->ram_bitmap->unsentmap, 0, ram_bitmap_pages); } }
Once there, rename the type to be shorter. Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 79 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 42 insertions(+), 37 deletions(-)