| Message ID | 20200421085300.7734-10-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migrate/ram: Fix resizing RAM blocks while migrating | expand |
* David Hildenbrand (david@redhat.com) wrote: > Let's consolidate resetting the variables. > > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > Cc: Juan Quintela <quintela@redhat.com> > Cc: Peter Xu <peterx@redhat.com> > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > migration/ram.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index 2a2165b478..7eca3165c8 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -3150,7 +3150,7 @@ static int ram_load_postcopy(QEMUFile *f) > /* Temporary page that is later 'placed' */ > void *postcopy_host_page = mis->postcopy_tmp_page; > void *host_page = NULL; > - bool all_zero = false; > + bool all_zero = true; > int target_pages = 0; > > while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) { > @@ -3176,7 +3176,6 @@ static int ram_load_postcopy(QEMUFile *f) > addr &= TARGET_PAGE_MASK; > > trace_ram_load_postcopy_loop((uint64_t)addr, flags); > - place_needed = false; > if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE | > RAM_SAVE_FLAG_COMPRESS_PAGE)) { > block = ram_block_from_stream(f, flags); > @@ -3204,9 +3203,7 @@ static int ram_load_postcopy(QEMUFile *f) > */ > page_buffer = postcopy_host_page + > host_page_offset_from_ram_block_offset(block, addr); > - /* If all TP are zero then we can optimise the place */ > if (target_pages == 1) { > - all_zero = true; > host_page = host_page_from_ram_block_offset(block, addr); > } else if (host_page != host_page_from_ram_block_offset(block, > addr)) { > @@ -3223,7 +3220,6 @@ static int ram_load_postcopy(QEMUFile *f) > */ > if (target_pages == (block->page_size / TARGET_PAGE_SIZE)) { > place_needed = true; > - target_pages = 0; > } > place_source = postcopy_host_page; > } > @@ -3300,6 +3296,10 @@ static int ram_load_postcopy(QEMUFile *f) > ret = postcopy_place_page(mis, host_page, place_source, > block); > } > + place_needed = false; > + target_pages = 0; > + /* Assume we have a zero page until we detect something different */ > + all_zero = true; > } > } > > -- > 2.25.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote: > * David Hildenbrand (david@redhat.com) wrote: > > Let's consolidate resetting the variables. > > > > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Cc: Juan Quintela <quintela@redhat.com> > > Cc: Peter Xu <peterx@redhat.com> > > Signed-off-by: David Hildenbrand <david@redhat.com> > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Queued this one only; I had to do manual patch application due to some renaming in 91ba442; but I think it's still OK. > > --- > > migration/ram.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/migration/ram.c b/migration/ram.c > > index 2a2165b478..7eca3165c8 100644 > > --- a/migration/ram.c > > +++ b/migration/ram.c > > @@ -3150,7 +3150,7 @@ static int ram_load_postcopy(QEMUFile *f) > > /* Temporary page that is later 'placed' */ > > void *postcopy_host_page = mis->postcopy_tmp_page; > > void *host_page = NULL; > > - bool all_zero = false; > > + bool all_zero = true; > > int target_pages = 0; > > > > while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) { > > @@ -3176,7 +3176,6 @@ static int ram_load_postcopy(QEMUFile *f) > > addr &= TARGET_PAGE_MASK; > > > > trace_ram_load_postcopy_loop((uint64_t)addr, flags); > > - place_needed = false; > > if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE | > > RAM_SAVE_FLAG_COMPRESS_PAGE)) { > > block = ram_block_from_stream(f, flags); > > @@ -3204,9 +3203,7 @@ static int ram_load_postcopy(QEMUFile *f) > > */ > > page_buffer = postcopy_host_page + > > host_page_offset_from_ram_block_offset(block, addr); > > - /* If all TP are zero then we can optimise the place */ > > if (target_pages == 1) { > > - all_zero = true; > > host_page = host_page_from_ram_block_offset(block, addr); > > } else if (host_page != host_page_from_ram_block_offset(block, > > addr)) { > > @@ -3223,7 +3220,6 @@ static int ram_load_postcopy(QEMUFile *f) > > */ > > if (target_pages == (block->page_size / TARGET_PAGE_SIZE)) { > > place_needed = true; > > - target_pages = 0; > > } > > place_source = postcopy_host_page; > > } > > @@ -3300,6 +3296,10 @@ static int ram_load_postcopy(QEMUFile *f) > > ret = postcopy_place_page(mis, host_page, place_source, > > block); > > } > > + place_needed = false; > > + target_pages = 0; > > + /* Assume we have a zero page until we detect something different */ > > + all_zero = true; > > } > > } > > > > -- > > 2.25.1 > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 07.05.20 17:42, Dr. David Alan Gilbert wrote: > * Dr. David Alan Gilbert (dgilbert@redhat.com) wrote: >> * David Hildenbrand (david@redhat.com) wrote: >>> Let's consolidate resetting the variables. >>> >>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> >>> Cc: Juan Quintela <quintela@redhat.com> >>> Cc: Peter Xu <peterx@redhat.com> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > Queued this one only; I had to do manual patch application due to some > renaming in 91ba442; but I think it's still OK. Thanks, how to best proceed with the other 12 patches in this series?
* David Hildenbrand (david@redhat.com) wrote: > On 07.05.20 17:42, Dr. David Alan Gilbert wrote: > > * Dr. David Alan Gilbert (dgilbert@redhat.com) wrote: > >> * David Hildenbrand (david@redhat.com) wrote: > >>> Let's consolidate resetting the variables. > >>> > >>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > >>> Cc: Juan Quintela <quintela@redhat.com> > >>> Cc: Peter Xu <peterx@redhat.com> > >>> Signed-off-by: David Hildenbrand <david@redhat.com> > >> > >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > > Queued this one only; I had to do manual patch application due to some > > renaming in 91ba442; but I think it's still OK. > > Thanks, how to best proceed with the other 12 patches in this series? Yeh I need to look at some of them; but they're buried in the other ~200 outstanding mails in my inbox. Dave > > -- > Thanks, > > David / dhildenb -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 07.05.20 21:00, Dr. David Alan Gilbert wrote: > * David Hildenbrand (david@redhat.com) wrote: >> On 07.05.20 17:42, Dr. David Alan Gilbert wrote: >>> * Dr. David Alan Gilbert (dgilbert@redhat.com) wrote: >>>> * David Hildenbrand (david@redhat.com) wrote: >>>>> Let's consolidate resetting the variables. >>>>> >>>>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> >>>>> Cc: Juan Quintela <quintela@redhat.com> >>>>> Cc: Peter Xu <peterx@redhat.com> >>>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>> >>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >>> >>> Queued this one only; I had to do manual patch application due to some >>> renaming in 91ba442; but I think it's still OK. >> >> Thanks, how to best proceed with the other 12 patches in this series? > > Yeh I need to look at some of them; but they're buried in the other ~200 > outstanding mails in my inbox. > Okay, let me know when you need a rebase/resend. Thanks!
diff --git a/migration/ram.c b/migration/ram.c index 2a2165b478..7eca3165c8 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -3150,7 +3150,7 @@ static int ram_load_postcopy(QEMUFile *f) /* Temporary page that is later 'placed' */ void *postcopy_host_page = mis->postcopy_tmp_page; void *host_page = NULL; - bool all_zero = false; + bool all_zero = true; int target_pages = 0; while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) { @@ -3176,7 +3176,6 @@ static int ram_load_postcopy(QEMUFile *f) addr &= TARGET_PAGE_MASK; trace_ram_load_postcopy_loop((uint64_t)addr, flags); - place_needed = false; if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE | RAM_SAVE_FLAG_COMPRESS_PAGE)) { block = ram_block_from_stream(f, flags); @@ -3204,9 +3203,7 @@ static int ram_load_postcopy(QEMUFile *f) */ page_buffer = postcopy_host_page + host_page_offset_from_ram_block_offset(block, addr); - /* If all TP are zero then we can optimise the place */ if (target_pages == 1) { - all_zero = true; host_page = host_page_from_ram_block_offset(block, addr); } else if (host_page != host_page_from_ram_block_offset(block, addr)) { @@ -3223,7 +3220,6 @@ static int ram_load_postcopy(QEMUFile *f) */ if (target_pages == (block->page_size / TARGET_PAGE_SIZE)) { place_needed = true; - target_pages = 0; } place_source = postcopy_host_page; } @@ -3300,6 +3296,10 @@ static int ram_load_postcopy(QEMUFile *f) ret = postcopy_place_page(mis, host_page, place_source, block); } + place_needed = false; + target_pages = 0; + /* Assume we have a zero page until we detect something different */ + all_zero = true; } }
Let's consolidate resetting the variables. Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Cc: Juan Quintela <quintela@redhat.com> Cc: Peter Xu <peterx@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- migration/ram.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)