Re: [Qemu-devel] [PATCH 06/15] postcopy: Record largest page size

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Record the largest page size in use; we'll need it soon for allocating
> > temporary buffers.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Not that I object, but .... could we store this in ram_list, and update it
> it at RAMBlock creation time?  Why searh for the value later when we can
> store it from the beggining.  Instead of putting it on migration_state,
> put it on the ram_list itself?
> 

We could, but the code does get quite a bit more complicated for little gain,
given that we currently read it exactly once.
The update at creation time would be easier, but then you have to also
update at deletion time and that has to run along the list just like this.
(or cache based on the ram_list.version)

Dave

> Later, Juan.
> 
> 
> > ---
> >  exec.c                        | 13 +++++++++++++
> >  include/exec/cpu-common.h     |  1 +
> >  include/migration/migration.h |  1 +
> >  migration/migration.c         |  1 +
> >  4 files changed, 16 insertions(+)
> >
> > diff --git a/exec.c b/exec.c
> > index 8d4bb0e..69331d0 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1485,6 +1485,19 @@ size_t qemu_ram_pagesize(RAMBlock *rb)
> >      return rb->page_size;
> >  }
> >  
> > +/* Returns the largest size of page in use */
> > +size_t qemu_ram_pagesize_largest(void)
> > +{
> > +    RAMBlock *block;
> > +    size_t largest = 0;
> > +
> > +    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> > +        largest = MAX(largest, qemu_ram_pagesize(block));
> > +    }
> > +
> > +    return largest;
> > +}
> > +
> >  static int memory_try_enable_merging(void *addr, size_t len)
> >  {
> >      if (!machine_mem_merge(current_machine)) {
> > diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> > index bd15853..0e67727 100644
> > --- a/include/exec/cpu-common.h
> > +++ b/include/exec/cpu-common.h
> > @@ -64,6 +64,7 @@ void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
> >  void qemu_ram_unset_idstr(RAMBlock *block);
> >  const char *qemu_ram_get_idstr(RAMBlock *rb);
> >  size_t qemu_ram_pagesize(RAMBlock *block);
> > +size_t qemu_ram_pagesize_largest(void);
> >  
> >  void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
> >                              int len, int is_write);
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 0188bcf..7b311dd 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -89,6 +89,7 @@ struct MigrationIncomingState {
> >       */
> >      QemuEvent main_thread_load_event;
> >  
> > +    size_t         largest_page_size;
> >      bool           have_fault_thread;
> >      QemuThread     fault_thread;
> >      QemuSemaphore  fault_thread_sem;
> > diff --git a/migration/migration.c b/migration/migration.c
> > index f498ab8..d8dafde 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -387,6 +387,7 @@ static void process_incoming_migration_co(void *opaque)
> >      int ret;
> >  
> >      mis = migration_incoming_state_new(f);
> > +    mis->largest_page_size = qemu_ram_pagesize_largest();
> >      postcopy_state_set(POSTCOPY_INCOMING_NONE);
> >      migrate_set_state(&mis->state, MIGRATION_STATUS_NONE,
> >                        MIGRATION_STATUS_ACTIVE);
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK