Re: [PATCH v4 09/13] migration/ram: Consolidate variable reset after placement in ram_load_postcopy()
On 07.05.20 21:00, Dr. David Alan Gilbert wrote: > * David Hildenbrand (da...@redhat.com) wrote: >> On 07.05.20 17:42, Dr. David Alan Gilbert wrote: >>> * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote: * David Hildenbrand (da...@redhat.com) wrote: > Let's consolidate resetting the variables. > > Cc: "Dr. David Alan Gilbert" > Cc: Juan Quintela > Cc: Peter Xu > Signed-off-by: David Hildenbrand Reviewed-by: Dr. David Alan Gilbert >>> >>> 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! -- Thanks, David / dhildenb
Re: [PATCH v4 09/13] migration/ram: Consolidate variable reset after placement in ram_load_postcopy()
* David Hildenbrand (da...@redhat.com) wrote: > On 07.05.20 17:42, Dr. David Alan Gilbert wrote: > > * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote: > >> * David Hildenbrand (da...@redhat.com) wrote: > >>> Let's consolidate resetting the variables. > >>> > >>> Cc: "Dr. David Alan Gilbert" > >>> Cc: Juan Quintela > >>> Cc: Peter Xu > >>> Signed-off-by: David Hildenbrand > >> > >> Reviewed-by: Dr. David Alan Gilbert > > > > 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 / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v4 09/13] migration/ram: Consolidate variable reset after placement in ram_load_postcopy()
On 07.05.20 17:42, Dr. David Alan Gilbert wrote: > * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote: >> * David Hildenbrand (da...@redhat.com) wrote: >>> Let's consolidate resetting the variables. >>> >>> Cc: "Dr. David Alan Gilbert" >>> Cc: Juan Quintela >>> Cc: Peter Xu >>> Signed-off-by: David Hildenbrand >> >> Reviewed-by: Dr. David Alan Gilbert > > 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? -- Thanks, David / dhildenb
Re: [PATCH v4 09/13] migration/ram: Consolidate variable reset after placement in ram_load_postcopy()
* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote: > * David Hildenbrand (da...@redhat.com) wrote: > > Let's consolidate resetting the variables. > > > > Cc: "Dr. David Alan Gilbert" > > Cc: Juan Quintela > > Cc: Peter Xu > > Signed-off-by: David Hildenbrand > > Reviewed-by: Dr. David Alan Gilbert 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 / dgilb...@redhat.com / Manchester, UK > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v4 09/13] migration/ram: Consolidate variable reset after placement in ram_load_postcopy()
* David Hildenbrand (da...@redhat.com) wrote: > Let's consolidate resetting the variables. > > Cc: "Dr. David Alan Gilbert" > Cc: Juan Quintela > Cc: Peter Xu > Signed-off-by: David Hildenbrand Reviewed-by: Dr. David Alan Gilbert > --- > 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 / dgilb...@redhat.com / Manchester, UK