diff mbox

[2/2] vl: Ensure the cpu_synchronize_all_post_init func in the appropriate location

Message ID 1484664152-24446-3-git-send-email-douly.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

Dou Liyang Jan. 17, 2017, 2:42 p.m. UTC
As the commit a4088c3eecb5f said, In the cpu_synchronize_all_post_init(),
we also 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.

The patch move the numa_post_machine_init func in the appropriate
location.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Eduardo Habkost Jan. 17, 2017, 8:21 p.m. UTC | #1
On Tue, Jan 17, 2017 at 10:42:32PM +0800, Dou Liyang wrote:
> As the commit a4088c3eecb5f said, In the cpu_synchronize_all_post_init(),
> we also use CPU_FOREACH macro to set all CPUs' namu_node.

I can't find commit a4088c3eecb5f, is it the commit ID of your
previous patch on your local tree? I don't see any NUMA-related
code triggered cpu_synchronize_all_post_init().

>                                                           So, we should
> make sure that we call it after Qemu has already initialied all the CPUs.

> 
> The patch move the numa_post_machine_init func in the appropriate
> location.

This patch moves cpu_synchronize_all_post_init(), not
numa_post_machine_init(). Could you describe which bug you are
fixing, exactly? It doesn't seem to be related to NUMA.

> 
> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> ---
>  vl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index f38b0e2..75adc2a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4547,8 +4547,6 @@ int main(int argc, char **argv, char **envp)
>  
>      audio_init();
>  
> -    cpu_synchronize_all_post_init();
> -
>      if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
>                            parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
>          exit(1);
> @@ -4570,6 +4568,8 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    cpu_synchronize_all_post_init();
> +
>      numa_post_machine_init();
>  
>      rom_reset_order_override();
> -- 
> 2.5.5
> 
> 
>
Dou Liyang Jan. 18, 2017, 2:17 a.m. UTC | #2
Hi, Eduardo

To clarify:

This patch is no related to the bug about NUMA.

This patch makes sure that all CPUs can run cpu state synchronization
on target vcpu thread, not just the CPUs which created by "-smp 2..."
at the beginning.

It is also caused by the execution sequence, So I put it here.

At 01/18/2017 04:21 AM, Eduardo Habkost wrote:
> On Tue, Jan 17, 2017 at 10:42:32PM +0800, Dou Liyang wrote:
>> As the commit a4088c3eecb5f said, In the cpu_synchronize_all_post_init(),
>> we also use CPU_FOREACH macro to set all CPUs' namu_node.
>
> I can't find commit a4088c3eecb5f, is it the commit ID of your
> previous patch on your local tree? I don't see any NUMA-related
> code triggered cpu_synchronize_all_post_init().
>

Oops, yes. It is the previous patch 1 in my local tree. I will modify
it.

>>                                                           So, we should
>> make sure that we call it after Qemu has already initialied all the CPUs.
>
>>
>> The patch move the numa_post_machine_init func in the appropriate
>> location.
>
> This patch moves cpu_synchronize_all_post_init(), not
> numa_post_machine_init(). Could you describe which bug you are
> fixing, exactly? It doesn't seem to be related to NUMA.
>

Yes, I will describe it exactly in the next version.

Thanks,
	Liyang
>>
>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>> ---
>>  vl.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index f38b0e2..75adc2a 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -4547,8 +4547,6 @@ int main(int argc, char **argv, char **envp)
>>
>>      audio_init();
>>
>> -    cpu_synchronize_all_post_init();
>> -
>>      if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
>>                            parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
>>          exit(1);
>> @@ -4570,6 +4568,8 @@ int main(int argc, char **argv, char **envp)
>>          exit(1);
>>      }
>>
>> +    cpu_synchronize_all_post_init();
>> +
>>      numa_post_machine_init();
>>
>>      rom_reset_order_override();
>> --
>> 2.5.5
>>
>>
>>
>
diff mbox

Patch

diff --git a/vl.c b/vl.c
index f38b0e2..75adc2a 100644
--- a/vl.c
+++ b/vl.c
@@ -4547,8 +4547,6 @@  int main(int argc, char **argv, char **envp)
 
     audio_init();
 
-    cpu_synchronize_all_post_init();
-
     if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
                           parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
         exit(1);
@@ -4570,6 +4568,8 @@  int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    cpu_synchronize_all_post_init();
+
     numa_post_machine_init();
 
     rom_reset_order_override();