* Wei Yang (richardw.y...@linux.intel.com) wrote: > After cleanup, it would be clear to audience there are two cases > ram_load: > > * precopy > * postcopy > > And it is not necessary to check postcopy_running on each iteration for > precopy. > > Signed-off-by: Wei Yang <richardw.y...@linux.intel.com> > --- > migration/ram.c | 73 +++++++++++++++++++++++++++++++------------------ > 1 file changed, 46 insertions(+), 27 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index 6bfdfae16e..5f6f07b255 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -4200,40 +4200,26 @@ static void colo_flush_ram_cache(void) > trace_colo_flush_ram_cache_end(); > } > > -static int ram_load(QEMUFile *f, void *opaque, int version_id) > +/** > + * ram_load_precopy: load a page in precopy case
This comment is wrong - although I realise you copied it from the postcopy case; they don't just load a single page; they load 'pages' Other than that, I think it's OK, so: Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > + * Returns 0 for success or -errno in case of error > + * > + * Called in precopy mode by ram_load(). > + * rcu_read_lock is taken prior to this being called. > + * > + * @f: QEMUFile where to send the data > + */ > +static int ram_load_precopy(QEMUFile *f) > { > - int flags = 0, ret = 0, invalid_flags = 0; > - static uint64_t seq_iter; > - int len = 0; > - /* > - * If system is running in postcopy mode, page inserts to host memory > must > - * be atomic > - */ > - bool postcopy_running = postcopy_is_running(); > + int flags = 0, ret = 0, invalid_flags = 0, len = 0; > /* ADVISE is earlier, it shows the source has the postcopy capability on > */ > bool postcopy_advised = postcopy_is_advised(); > - > - seq_iter++; > - > - if (version_id != 4) { > - return -EINVAL; > - } > - > if (!migrate_use_compression()) { > invalid_flags |= RAM_SAVE_FLAG_COMPRESS_PAGE; > } > - /* This RCU critical section can be very long running. > - * When RCU reclaims in the code start to become numerous, > - * it will be necessary to reduce the granularity of this > - * critical section. > - */ > - rcu_read_lock(); > - > - if (postcopy_running) { > - ret = ram_load_postcopy(f); > - } > > - while (!postcopy_running && !ret && !(flags & RAM_SAVE_FLAG_EOS)) { > + while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) { > ram_addr_t addr, total_ram_bytes; > void *host = NULL; > uint8_t ch; > @@ -4390,6 +4376,39 @@ static int ram_load(QEMUFile *f, void *opaque, int > version_id) > } > } > > + return ret; > +} > + > +static int ram_load(QEMUFile *f, void *opaque, int version_id) > +{ > + int ret = 0; > + static uint64_t seq_iter; > + /* > + * If system is running in postcopy mode, page inserts to host memory > must > + * be atomic > + */ > + bool postcopy_running = postcopy_is_running(); > + > + seq_iter++; > + > + if (version_id != 4) { > + return -EINVAL; > + } > + > + /* > + * This RCU critical section can be very long running. > + * When RCU reclaims in the code start to become numerous, > + * it will be necessary to reduce the granularity of this > + * critical section. > + */ > + rcu_read_lock(); > + > + if (postcopy_running) { > + ret = ram_load_postcopy(f); > + } else { > + ret = ram_load_precopy(f); > + } > + > ret |= wait_for_decompress_done(); > rcu_read_unlock(); > trace_ram_load_complete(ret, seq_iter); > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK