Re: [Qemu-devel] [PATCH v3.2 01/31] NUMA: move numa related code to new file numa.c

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jun 09, 2014 at 10:22:35AM +0800, Hu Tao wrote:
> On Sun, Jun 08, 2014 at 01:09:03PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 14, 2014 at 05:43:05PM +0800, Hu Tao wrote:
> > > From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> > > 
> > > Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > 
> > This is also based on bits by Blue Swirl and
> > Andre Przywara, I think you should add their S.O.B. lines
> > from the original commits, just in case we need to locate the authors
> > down the line.
> 
> Sure. Should I use Andre's new email address(linaro) as the old
> email address(amd) can't be reached now?

I used the S.O.B. line that was used when code was submitted.

> > 
> > > ---
> > >  Makefile.target           |   2 +-
> > >  cpus.c                    |  14 ----
> > >  include/exec/cpu-all.h    |   2 -
> > >  include/exec/cpu-common.h |   2 +
> > >  include/sysemu/cpus.h     |   1 -
> > >  include/sysemu/sysemu.h   |   3 +
> > >  numa.c                    | 186 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  vl.c                      | 139 +---------------------------------
> > >  8 files changed, 193 insertions(+), 156 deletions(-)
> > >  create mode 100644 numa.c
> > > 
> > > diff --git a/Makefile.target b/Makefile.target
> > > index 9986047..dd815bb 100644
> > > --- a/Makefile.target
> > > +++ b/Makefile.target
> > > @@ -112,7 +112,7 @@ endif #CONFIG_BSD_USER
> > >  #########################################################
> > >  # System emulator target
> > >  ifdef CONFIG_SOFTMMU
> > > -obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o
> > > +obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o
> > >  obj-y += qtest.o
> > >  obj-y += hw/
> > >  obj-$(CONFIG_FDT) += device_tree.o
> > > diff --git a/cpus.c b/cpus.c
> > > index 7bbe153..7f87adb 100644
> > > --- a/cpus.c
> > > +++ b/cpus.c
> > > @@ -1313,20 +1313,6 @@ static void tcg_exec_all(void)
> > >      exit_request = 0;
> > >  }
> > >  
> > > -void set_numa_modes(void)
> > > -{
> > > -    CPUState *cpu;
> > > -    int i;
> > > -
> > > -    CPU_FOREACH(cpu) {
> > > -        for (i = 0; i < nb_numa_nodes; i++) {
> > > -            if (test_bit(cpu->cpu_index, node_cpumask[i])) {
> > > -                cpu->numa_node = i;
> > > -            }
> > > -        }
> > > -    }
> > > -}
> > > -
> > >  void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
> > >  {
> > >      /* XXX: implement xxx_cpu_list for targets that still miss it */
> > > diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> > > index fb649a4..758a928 100644
> > > --- a/include/exec/cpu-all.h
> > > +++ b/include/exec/cpu-all.h
> > > @@ -414,8 +414,6 @@ CPUArchState *cpu_copy(CPUArchState *env);
> > >  
> > >  /* memory API */
> > >  
> > > -extern ram_addr_t ram_size;
> > > -
> > >  /* RAM is pre-allocated and passed into qemu_ram_alloc_from_ptr */
> > >  #define RAM_PREALLOC_MASK   (1 << 0)
> > >  
> > > diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> > > index a21b65a..e8c7970 100644
> > > --- a/include/exec/cpu-common.h
> > > +++ b/include/exec/cpu-common.h
> > > @@ -45,6 +45,8 @@ typedef uintptr_t ram_addr_t;
> > >  #  define RAM_ADDR_FMT "%" PRIxPTR
> > >  #endif
> > >  
> > > +extern ram_addr_t ram_size;
> > > +
> > >  /* memory API */
> > >  
> > >  typedef void CPUWriteMemoryFunc(void *opaque, hwaddr addr, uint32_t value);
> > > diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
> > > index 6502488..4f79081 100644
> > > --- a/include/sysemu/cpus.h
> > > +++ b/include/sysemu/cpus.h
> > > @@ -23,7 +23,6 @@ extern int smp_threads;
> > >  #define smp_threads 1
> > >  #endif
> > >  
> > > -void set_numa_modes(void);
> > >  void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg);
> > >  
> > >  #endif
> > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > > index ba5c7f8..565c8f6 100644
> > > --- a/include/sysemu/sysemu.h
> > > +++ b/include/sysemu/sysemu.h
> > > @@ -144,6 +144,9 @@ extern QEMUClockType rtc_clock;
> > >  extern int nb_numa_nodes;
> > >  extern uint64_t node_mem[MAX_NODES];
> > >  extern unsigned long *node_cpumask[MAX_NODES];
> > > +void numa_add(const char *optarg);
> > > +void set_numa_nodes(void);
> > > +void set_numa_modes(void);
> > >  
> > >  #define MAX_OPTION_ROMS 16
> > >  typedef struct QEMUOptionRom {
> > 
> > Down the line I think numa.h would be nicer,
> > not a must to fix.
> > 
> > 
> > 
> > > diff --git a/numa.c b/numa.c
> > > new file mode 100644
> > > index 0000000..395c14f
> > > --- /dev/null
> > > +++ b/numa.c
> > > @@ -0,0 +1,186 @@
> > > +/*
> > > + * QEMU System Emulator
> > 
> > Is this the best we can do here? This was reasonable for vl.c
> > 
> > Better  "NUMA parameter parsing"
> 
> Sure.
> 
> > 
> > > + *
> > > + * Copyright (c) 2013 Fujitsu Ltd.
> > 
> > It's 2014 isn't it?
> > 
> > 
> > > + * Author: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> > 
> > These tags are probably best avoided for non original code:
> > we don't have the resources to keep them up to date as code changes.
> > Better dropped it for now.
> 
> OK.
> 
> 
> Regards,
> Hu