* [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not @ 2017-08-14 10:11 Dou Liyang 2017-08-14 10:20 ` Dou Liyang 2017-08-14 12:44 ` Eduardo Habkost 0 siblings, 2 replies; 6+ messages in thread From: Dou Liyang @ 2017-08-14 10:11 UTC (permalink / raw) To: qemu-devel; +Cc: ehabkost, imammedo, dgilbert, armbru, david, Dou Liyang Currently, Using the fisrt node without memory on the machine makes QEMU unhappy. With this example command line: ... \ -m 1024M,slots=4,maxmem=32G \ -numa node,nodeid=0 \ -numa node,mem=1024M,nodeid=1 \ -numa node,nodeid=2 \ -numa node,nodeid=3 \ Guest reports "No NUMA configuration found" and the NUMA topology is wrong. This is because when QEMU builds ACPI SRAT, it regards node0 as the default node to deal with the memory hole(640K-1M). this means the node0 must have some memory(>1M) firstly. Add a check in parse_numa_opts to avoid this situation. Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> --- numa.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/numa.c b/numa.c index e32af04..1d6f73f 100644 --- a/numa.c +++ b/numa.c @@ -464,6 +464,10 @@ void parse_numa_opts(MachineState *ms) if (i == nb_numa_nodes) { assert(mc->numa_auto_assign_ram); mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size); + } else if (i != 0) { + error_report("The first NUMA node must have some memory" + " for building ACPI SART"); + exit(1); } numa_total = 0; -- 2.5.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not 2017-08-14 10:11 [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not Dou Liyang @ 2017-08-14 10:20 ` Dou Liyang 2017-08-14 12:44 ` Eduardo Habkost 1 sibling, 0 replies; 6+ messages in thread From: Dou Liyang @ 2017-08-14 10:20 UTC (permalink / raw) To: qemu-devel Cc: ehabkost, imammedo, dgilbert, armbru, david, Michael S. Tsirkin I'm sorry, forgot to cc Michael S. Tsirkin At 08/14/2017 06:11 PM, Dou Liyang wrote: > Currently, Using the fisrt node without memory on the machine makes > QEMU unhappy. With this example command line: > ... \ > -m 1024M,slots=4,maxmem=32G \ > -numa node,nodeid=0 \ > -numa node,mem=1024M,nodeid=1 \ > -numa node,nodeid=2 \ > -numa node,nodeid=3 \ > Guest reports "No NUMA configuration found" and the NUMA topology is > wrong. > > This is because when QEMU builds ACPI SRAT, it regards node0 as the > default node to deal with the memory hole(640K-1M). this means the > node0 must have some memory(>1M) firstly. > > Add a check in parse_numa_opts to avoid this situation. > > Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> > --- > numa.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/numa.c b/numa.c > index e32af04..1d6f73f 100644 > --- a/numa.c > +++ b/numa.c > @@ -464,6 +464,10 @@ void parse_numa_opts(MachineState *ms) > if (i == nb_numa_nodes) { > assert(mc->numa_auto_assign_ram); > mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size); > + } else if (i != 0) { > + error_report("The first NUMA node must have some memory" > + " for building ACPI SART"); > + exit(1); > } > > numa_total = 0; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not 2017-08-14 10:11 [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not Dou Liyang 2017-08-14 10:20 ` Dou Liyang @ 2017-08-14 12:44 ` Eduardo Habkost 2017-08-15 1:26 ` Dou Liyang 1 sibling, 1 reply; 6+ messages in thread From: Eduardo Habkost @ 2017-08-14 12:44 UTC (permalink / raw) To: Dou Liyang; +Cc: qemu-devel, imammedo, dgilbert, armbru, david On Mon, Aug 14, 2017 at 06:11:11PM +0800, Dou Liyang wrote: > Currently, Using the fisrt node without memory on the machine makes > QEMU unhappy. With this example command line: > ... \ > -m 1024M,slots=4,maxmem=32G \ > -numa node,nodeid=0 \ > -numa node,mem=1024M,nodeid=1 \ > -numa node,nodeid=2 \ > -numa node,nodeid=3 \ > Guest reports "No NUMA configuration found" and the NUMA topology is > wrong. > > This is because when QEMU builds ACPI SRAT, it regards node0 as the > default node to deal with the memory hole(640K-1M). this means the > node0 must have some memory(>1M) firstly. > > Add a check in parse_numa_opts to avoid this situation. > > Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> > --- > numa.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/numa.c b/numa.c > index e32af04..1d6f73f 100644 > --- a/numa.c > +++ b/numa.c > @@ -464,6 +464,10 @@ void parse_numa_opts(MachineState *ms) > if (i == nb_numa_nodes) { > assert(mc->numa_auto_assign_ram); > mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size); > + } else if (i != 0) { > + error_report("The first NUMA node must have some memory" > + " for building ACPI SART"); > + exit(1); This doesn't belong to numa.c. numa.c is generic code, and the requirement you described is specific for PC. Anyway, adding this check would make existing VM configurations refuse to run after a QEMU upgrade. I suggest fixing the bug in the ACPI code instead. -- Eduardo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not 2017-08-14 12:44 ` Eduardo Habkost @ 2017-08-15 1:26 ` Dou Liyang 2017-08-15 8:04 ` Igor Mammedov 0 siblings, 1 reply; 6+ messages in thread From: Dou Liyang @ 2017-08-15 1:26 UTC (permalink / raw) To: Eduardo Habkost Cc: qemu-devel, imammedo, dgilbert, armbru, david, Michael S. Tsirkin Hi Eduardo, At 08/14/2017 08:44 PM, Eduardo Habkost wrote: > On Mon, Aug 14, 2017 at 06:11:11PM +0800, Dou Liyang wrote: >> Currently, Using the fisrt node without memory on the machine makes >> QEMU unhappy. With this example command line: >> ... \ >> -m 1024M,slots=4,maxmem=32G \ >> -numa node,nodeid=0 \ >> -numa node,mem=1024M,nodeid=1 \ >> -numa node,nodeid=2 \ >> -numa node,nodeid=3 \ >> Guest reports "No NUMA configuration found" and the NUMA topology is >> wrong. >> >> This is because when QEMU builds ACPI SRAT, it regards node0 as the >> default node to deal with the memory hole(640K-1M). this means the >> node0 must have some memory(>1M) firstly. >> >> Add a check in parse_numa_opts to avoid this situation. >> >> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> >> --- >> numa.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/numa.c b/numa.c >> index e32af04..1d6f73f 100644 >> --- a/numa.c >> +++ b/numa.c >> @@ -464,6 +464,10 @@ void parse_numa_opts(MachineState *ms) >> if (i == nb_numa_nodes) { >> assert(mc->numa_auto_assign_ram); >> mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size); >> + } else if (i != 0) { >> + error_report("The first NUMA node must have some memory" >> + " for building ACPI SART"); >> + exit(1); > > This doesn't belong to numa.c. numa.c is generic code, and the > requirement you described is specific for PC. > > Anyway, adding this check would make existing VM configurations > refuse to run after a QEMU upgrade. I suggest fixing the bug in > the ACPI code instead. > I see. If fixing the bug in the ACPI code, I have two solutions: 1). Add a check in build_srat(). If the first node has no memory, QEMU will exit. 2). Fix the initialization of memory affinity structure to cover this situation. Using the first node which has memory to deal with the memory hole. I prefer solution 2. what about you? Thanks, dou. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not 2017-08-15 1:26 ` Dou Liyang @ 2017-08-15 8:04 ` Igor Mammedov 2017-08-15 9:04 ` Dou Liyang 0 siblings, 1 reply; 6+ messages in thread From: Igor Mammedov @ 2017-08-15 8:04 UTC (permalink / raw) To: Dou Liyang Cc: Eduardo Habkost, qemu-devel, dgilbert, armbru, david, Michael S. Tsirkin On Tue, 15 Aug 2017 09:26:46 +0800 Dou Liyang <douly.fnst@cn.fujitsu.com> wrote: > Hi Eduardo, > > At 08/14/2017 08:44 PM, Eduardo Habkost wrote: > > On Mon, Aug 14, 2017 at 06:11:11PM +0800, Dou Liyang wrote: > >> Currently, Using the fisrt node without memory on the machine makes > >> QEMU unhappy. With this example command line: > >> ... \ > >> -m 1024M,slots=4,maxmem=32G \ > >> -numa node,nodeid=0 \ > >> -numa node,mem=1024M,nodeid=1 \ > >> -numa node,nodeid=2 \ > >> -numa node,nodeid=3 \ > >> Guest reports "No NUMA configuration found" and the NUMA topology is > >> wrong. > >> > >> This is because when QEMU builds ACPI SRAT, it regards node0 as the > >> default node to deal with the memory hole(640K-1M). this means the > >> node0 must have some memory(>1M) firstly. > >> > >> Add a check in parse_numa_opts to avoid this situation. > >> > >> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> > >> --- > >> numa.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/numa.c b/numa.c > >> index e32af04..1d6f73f 100644 > >> --- a/numa.c > >> +++ b/numa.c > >> @@ -464,6 +464,10 @@ void parse_numa_opts(MachineState *ms) > >> if (i == nb_numa_nodes) { > >> assert(mc->numa_auto_assign_ram); > >> mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size); > >> + } else if (i != 0) { > >> + error_report("The first NUMA node must have some memory" > >> + " for building ACPI SART"); > >> + exit(1); > > > > This doesn't belong to numa.c. numa.c is generic code, and the > > requirement you described is specific for PC. > > > > Anyway, adding this check would make existing VM configurations > > refuse to run after a QEMU upgrade. I suggest fixing the bug in > > the ACPI code instead. > > > > I see. > > If fixing the bug in the ACPI code, I have two solutions: > > 1). Add a check in build_srat(). If the first node has no memory, QEMU > will exit. > > 2). Fix the initialization of memory affinity structure to cover this > situation. Using the first node which has memory to deal with the memory > hole. > > I prefer solution 2. what about you? I'd go for 2nd solution > > Thanks, > dou. > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not 2017-08-15 8:04 ` Igor Mammedov @ 2017-08-15 9:04 ` Dou Liyang 0 siblings, 0 replies; 6+ messages in thread From: Dou Liyang @ 2017-08-15 9:04 UTC (permalink / raw) To: Igor Mammedov Cc: Eduardo Habkost, qemu-devel, dgilbert, armbru, david, Michael S. Tsirkin Hi Igor, At 08/15/2017 04:04 PM, Igor Mammedov wrote: > On Tue, 15 Aug 2017 09:26:46 +0800 > Dou Liyang <douly.fnst@cn.fujitsu.com> wrote: > >> Hi Eduardo, >> >> At 08/14/2017 08:44 PM, Eduardo Habkost wrote: >>> On Mon, Aug 14, 2017 at 06:11:11PM +0800, Dou Liyang wrote: >>>> Currently, Using the fisrt node without memory on the machine makes >>>> QEMU unhappy. With this example command line: >>>> ... \ >>>> -m 1024M,slots=4,maxmem=32G \ >>>> -numa node,nodeid=0 \ >>>> -numa node,mem=1024M,nodeid=1 \ >>>> -numa node,nodeid=2 \ >>>> -numa node,nodeid=3 \ >>>> Guest reports "No NUMA configuration found" and the NUMA topology is >>>> wrong. >>>> >>>> This is because when QEMU builds ACPI SRAT, it regards node0 as the >>>> default node to deal with the memory hole(640K-1M). this means the >>>> node0 must have some memory(>1M) firstly. >>>> >>>> Add a check in parse_numa_opts to avoid this situation. >>>> >>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> >>>> --- >>>> numa.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/numa.c b/numa.c >>>> index e32af04..1d6f73f 100644 >>>> --- a/numa.c >>>> +++ b/numa.c >>>> @@ -464,6 +464,10 @@ void parse_numa_opts(MachineState *ms) >>>> if (i == nb_numa_nodes) { >>>> assert(mc->numa_auto_assign_ram); >>>> mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size); >>>> + } else if (i != 0) { >>>> + error_report("The first NUMA node must have some memory" >>>> + " for building ACPI SART"); >>>> + exit(1); >>> >>> This doesn't belong to numa.c. numa.c is generic code, and the >>> requirement you described is specific for PC. >>> >>> Anyway, adding this check would make existing VM configurations >>> refuse to run after a QEMU upgrade. I suggest fixing the bug in >>> the ACPI code instead. >>> >> >> I see. >> >> If fixing the bug in the ACPI code, I have two solutions: >> >> 1). Add a check in build_srat(). If the first node has no memory, QEMU >> will exit. >> >> 2). Fix the initialization of memory affinity structure to cover this >> situation. Using the first node which has memory to deal with the memory >> hole. >> >> I prefer solution 2. what about you? > I'd go for 2nd solution > Yeah, I see. And I just sent the 2nd solution, waiting for your reply. Thanks, dou. >> >> Thanks, >> dou. >> >> > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-15 9:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-14 10:11 [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not Dou Liyang 2017-08-14 10:20 ` Dou Liyang 2017-08-14 12:44 ` Eduardo Habkost 2017-08-15 1:26 ` Dou Liyang 2017-08-15 8:04 ` Igor Mammedov 2017-08-15 9:04 ` Dou Liyang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).