Re: [Qemu-devel] [PATCH 1/2] vl: Ensure the numa_post_machine_init func in the appropriate location

[Dou Liyang]

Hi, Eduardo

At 01/18/2017 04:09 AM, Eduardo Habkost wrote:
> On Tue, Jan 17, 2017 at 10:42:31PM +0800, Dou Liyang wrote:
> > In the numa_post_machine_init(), we use CPU_FOREACH macro to set all
> > CPUs' namu_node. So, we should make sure that we call it after Qemu
> > has already initialied all the CPUs.
> >
> > As we all know, the CPUs can be created by "-smp"(pc_new_cpu) or
> > "-device"(qdev_device_add) command. But, before the device init,
> > Qemu execute the numa_post_machine_init earlier. It makes the mapping
> > of NUMA nodes and CPUs incorrect.
> >
> > The patch move the numa_post_machine_init func in the appropriate
> > location.
> >
> > Signed-off-by: Dou Liyang <address@hidden>
>
> I would like to move cpu_index initialization to
> qom/cpu.c:cpu_common_realizefn(), and remove
> numa_post_machine_init() completely.

Thanks, it is a good idea. I will try it later.

But, I hope to know that:

If you may want to say the cpu->**numa_node** initialization?
Because the **cpu_index** initialization is in pc_cpu_pre_plug()
currently, like that: cs->cpu_index = idx;

OR

You want to move both of them in cpu_common_realizefn() ?

Thanks,
        Liyang.

 But this fixes the bug while
> we don't do that.
>
> Reviewed-by: Eduardo Habkost <address@hidden>
>
> Queued on numa-next. Thanks!
>
> > ---
> >  vl.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index c643d3f..f38b0e2 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -4549,8 +4549,6 @@ int main(int argc, char **argv, char **envp)
> >
> >      cpu_synchronize_all_post_init();
> >
> > -    numa_post_machine_init();
> > -
> >      if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
> >                            parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
> >          exit(1);
> > @@ -4571,6 +4569,9 @@ int main(int argc, char **argv, char **envp)
> >                            device_init_func, NULL, NULL)) {
> >          exit(1);
> >      }
> > +
> > +    numa_post_machine_init();
> > +
> >      rom_reset_order_override();
> >
> >      /* Did we create any drives that we failed to create a device for? */
> > --
> > 2.5.5