| Message ID | 1522938923-96058-2-git-send-email-spopovyc@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [for,2.13,1/2] Revert "spapr: Don't allow memory hotplug to memory less nodes" | expand |
On Thu, Apr 05, 2018 at 10:35:22AM -0400, Serhii Popovych wrote: > This reverts commit b556854bd8524c26b8be98ab1bfdf0826831e793. > > Leave change @node type from uint32_t to to int from reverted commit > because node < 0 is always false. > > Signed-off-by: Serhii Popovych <spopovyc@redhat.com> > --- > hw/ppc/spapr.c | 22 ---------------------- > 1 file changed, 22 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 2c0be8c..3ad4545 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3477,28 +3477,6 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev, > return; > } > > - /* > - * Currently PowerPC kernel doesn't allow hot-adding memory to > - * memory-less node, but instead will silently add the memory > - * to the first node that has some memory. This causes two > - * unexpected behaviours for the user. > - * > - * - Memory gets hotplugged to a different node than what the user > - * specified. > - * - Since pc-dimm subsystem in QEMU still thinks that memory belongs > - * to memory-less node, a reboot will set things accordingly > - * and the previously hotplugged memory now ends in the right node. > - * This appears as if some memory moved from one node to another. > - * > - * So until kernel starts supporting memory hotplug to memory-less > - * nodes, just prevent such attempts upfront in QEMU. > - */ > - if (nb_numa_nodes && !numa_info[node].node_mem) { > - error_setg(errp, "Can't hotplug memory to memory-less node %d", > - node); > - return; > - } > - If you remove this unconditionally, wouldn't it be a problem in case of newer QEMU with older guest kernels ? Regards, Bharata.
Bharata B Rao wrote: > On Thu, Apr 05, 2018 at 10:35:22AM -0400, Serhii Popovych wrote: >> This reverts commit b556854bd8524c26b8be98ab1bfdf0826831e793. >> >> Leave change @node type from uint32_t to to int from reverted commit >> because node < 0 is always false. >> >> Signed-off-by: Serhii Popovych <spopovyc@redhat.com> >> --- >> hw/ppc/spapr.c | 22 ---------------------- >> 1 file changed, 22 deletions(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 2c0be8c..3ad4545 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -3477,28 +3477,6 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev, >> return; >> } >> >> - /* >> - * Currently PowerPC kernel doesn't allow hot-adding memory to >> - * memory-less node, but instead will silently add the memory >> - * to the first node that has some memory. This causes two >> - * unexpected behaviours for the user. >> - * >> - * - Memory gets hotplugged to a different node than what the user >> - * specified. >> - * - Since pc-dimm subsystem in QEMU still thinks that memory belongs >> - * to memory-less node, a reboot will set things accordingly >> - * and the previously hotplugged memory now ends in the right node. >> - * This appears as if some memory moved from one node to another. >> - * >> - * So until kernel starts supporting memory hotplug to memory-less >> - * nodes, just prevent such attempts upfront in QEMU. >> - */ >> - if (nb_numa_nodes && !numa_info[node].node_mem) { >> - error_setg(errp, "Can't hotplug memory to memory-less node %d", >> - node); >> - return; >> - } >> - > > If you remove this unconditionally, wouldn't it be a problem in case > of newer QEMU with older guest kernels ? Yes, that definitely would affect guest kernels without such support. We probably need to add some capability to test for guest kernel functionality presence. > > Regards, > Bharata. > >
On Fri, Apr 06, 2018 at 08:48:55AM +0300, Serhii Popovych wrote: > Bharata B Rao wrote: > > On Thu, Apr 05, 2018 at 10:35:22AM -0400, Serhii Popovych wrote: > >> This reverts commit b556854bd8524c26b8be98ab1bfdf0826831e793. > >> > >> Leave change @node type from uint32_t to to int from reverted commit > >> because node < 0 is always false. > >> > >> Signed-off-by: Serhii Popovych <spopovyc@redhat.com> > >> --- > >> hw/ppc/spapr.c | 22 ---------------------- > >> 1 file changed, 22 deletions(-) > >> > >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > >> index 2c0be8c..3ad4545 100644 > >> --- a/hw/ppc/spapr.c > >> +++ b/hw/ppc/spapr.c > >> @@ -3477,28 +3477,6 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev, > >> return; > >> } > >> > >> - /* > >> - * Currently PowerPC kernel doesn't allow hot-adding memory to > >> - * memory-less node, but instead will silently add the memory > >> - * to the first node that has some memory. This causes two > >> - * unexpected behaviours for the user. > >> - * > >> - * - Memory gets hotplugged to a different node than what the user > >> - * specified. > >> - * - Since pc-dimm subsystem in QEMU still thinks that memory belongs > >> - * to memory-less node, a reboot will set things accordingly > >> - * and the previously hotplugged memory now ends in the right node. > >> - * This appears as if some memory moved from one node to another. > >> - * > >> - * So until kernel starts supporting memory hotplug to memory-less > >> - * nodes, just prevent such attempts upfront in QEMU. > >> - */ > >> - if (nb_numa_nodes && !numa_info[node].node_mem) { > >> - error_setg(errp, "Can't hotplug memory to memory-less node %d", > >> - node); > >> - return; > >> - } > >> - > > > > If you remove this unconditionally, wouldn't it be a problem in case > > of newer QEMU with older guest kernels ? > > Yes, that definitely would affect guest kernels without such support. We > probably need to add some capability to test for guest kernel > functionality presence. Hm, maybe. So first, we should check when the guest side support came in. If it's old enough we might not care. PAPR does include a mechanism for negotiating guest/host capabilities. However, I don't think it has a bit for this specific feature, so I can't really see a way to do this cleanly. I don't think we necessarily have to handle that case: it's not like we can reasonably workaround *every* possible guest bug/limitation from the host side. If you want to create a system with a memory-less node you need an OS that can handle that, nothing really special there.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 2c0be8c..3ad4545 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3477,28 +3477,6 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev, return; } - /* - * Currently PowerPC kernel doesn't allow hot-adding memory to - * memory-less node, but instead will silently add the memory - * to the first node that has some memory. This causes two - * unexpected behaviours for the user. - * - * - Memory gets hotplugged to a different node than what the user - * specified. - * - Since pc-dimm subsystem in QEMU still thinks that memory belongs - * to memory-less node, a reboot will set things accordingly - * and the previously hotplugged memory now ends in the right node. - * This appears as if some memory moved from one node to another. - * - * So until kernel starts supporting memory hotplug to memory-less - * nodes, just prevent such attempts upfront in QEMU. - */ - if (nb_numa_nodes && !numa_info[node].node_mem) { - error_setg(errp, "Can't hotplug memory to memory-less node %d", - node); - return; - } - spapr_memory_plug(hotplug_dev, dev, node, errp); } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) { spapr_core_plug(hotplug_dev, dev, errp);
This reverts commit b556854bd8524c26b8be98ab1bfdf0826831e793. Leave change @node type from uint32_t to to int from reverted commit because node < 0 is always false. Signed-off-by: Serhii Popovych <spopovyc@redhat.com> --- hw/ppc/spapr.c | 22 ---------------------- 1 file changed, 22 deletions(-)