| Message ID | 20160602140737.GB18628@in.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02.06.2016 16:07, Bharata B Rao wrote: > On Thu, Jun 02, 2016 at 09:03:15AM +0200, Thomas Huth wrote: >> On 02.06.2016 06:39, Bharata B Rao wrote: >> ... >>> Agreed. Here is the updated patch: >>> >>> spapr: Increase hotpluggable memory slots to 256 >>> >>> From: Bharata B Rao <bharata@linux.vnet.ibm.com> >>> >>> KVM now supports 512 memslots on PowerPC (earlier it was 32). Allow half >>> of it (256) to be used as hotpluggable memory slots. >>> >>> Instead of hard coding the max value, use the KVM supplied value if KVM >>> is enabled. Otherwise resort to the default value of 32. >>> >>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> >>> --- >>> hw/ppc/spapr.c | 15 +++++++++++++-- >>> 1 file changed, 13 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index 44e401a..c82adef 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -1816,11 +1816,22 @@ static void ppc_spapr_init(MachineState *machine) >>> /* initialize hotplug memory address space */ >>> if (machine->ram_size < machine->maxram_size) { >>> ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size; >>> + /* >>> + * Number of memslots supported by KVM on PowerPC was increased >>> + * from 32 to 512. Let us limit the number of hotpluggable slots >>> + * to half of that (256). However ensure that number of slots >>> + * doesn't drop below 32 on older hosts. >>> + */ >> >> Using "hard-coded" information like "increased to 512" in comments is >> true for the current state, but this has a risk of being out of date >> soon. Once we change the memslots in the kernel, this comment is not >> true anymore and might cause confusion. Better talk about leaving half >> of the kernel memslots for PCI and other devices, or so. > > Just want to note that even though we are limiting hotpluggable memory > slots to half of max, it is always possible for other devices to eat > into the memory hotplug slots, right ? Right. But 256 slots for those other devices is already plenty, I think. Let's hope that it is enough for a while - if it's not enough, we've got to rework the kernel code again (and maybe switch to dynamic memslots allocation instead). Thomas
On Thu, Jun 02, 2016 at 07:37:37PM +0530, Bharata B Rao wrote: > On Thu, Jun 02, 2016 at 09:03:15AM +0200, Thomas Huth wrote: > > On 02.06.2016 06:39, Bharata B Rao wrote: > > ... > > > Agreed. Here is the updated patch: > > > > > > spapr: Increase hotpluggable memory slots to 256 > > > > > > From: Bharata B Rao <bharata@linux.vnet.ibm.com> > > > > > > KVM now supports 512 memslots on PowerPC (earlier it was 32). Allow half > > > of it (256) to be used as hotpluggable memory slots. > > > > > > Instead of hard coding the max value, use the KVM supplied value if KVM > > > is enabled. Otherwise resort to the default value of 32. > > > > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > > --- > > > hw/ppc/spapr.c | 15 +++++++++++++-- > > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index 44e401a..c82adef 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -1816,11 +1816,22 @@ static void ppc_spapr_init(MachineState *machine) > > > /* initialize hotplug memory address space */ > > > if (machine->ram_size < machine->maxram_size) { > > > ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size; > > > + /* > > > + * Number of memslots supported by KVM on PowerPC was increased > > > + * from 32 to 512. Let us limit the number of hotpluggable slots > > > + * to half of that (256). However ensure that number of slots > > > + * doesn't drop below 32 on older hosts. > > > + */ > > > > Using "hard-coded" information like "increased to 512" in comments is > > true for the current state, but this has a risk of being out of date > > soon. Once we change the memslots in the kernel, this comment is not > > true anymore and might cause confusion. Better talk about leaving half > > of the kernel memslots for PCI and other devices, or so. > > Just want to note that even though we are limiting hotpluggable memory > slots to half of max, it is always possible for other devices to eat > into the memory hotplug slots, right ? > > David - here is the patch updated with comments as suggested by Thomas. > If you need a separate post, let me know. Applied to ppc-for-2.7, thanks. > > spapr: Increase hotpluggable memory slots to 256 > > From: Bharata B Rao <bharata@linux.vnet.ibm.com> > > KVM now supports 512 memslots on PowerPC (earlier it was 32). Allow half > of it (256) to be used as hotpluggable memory slots. > > Instead of hard coding the max value, use the KVM supplied value if KVM > is enabled. Otherwise resort to the default value of 32. > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > Reviewed-by: Thomas Huth <thuth@redhat.com> > --- > hw/ppc/spapr.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 44e401a..14cc6ae 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1816,11 +1816,21 @@ static void ppc_spapr_init(MachineState *machine) > /* initialize hotplug memory address space */ > if (machine->ram_size < machine->maxram_size) { > ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size; > + /* > + * Limit the number of hotpluggable memory slots to half the number > + * slots that KVM supports, leaving the other half for PCI and other > + * devices. However ensure that number of slots doesn't drop below 32. > + */ > + int max_memslots = kvm_enabled() ? kvm_get_max_memslots() / 2 : > + SPAPR_MAX_RAM_SLOTS; > > - if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) { > + if (max_memslots < SPAPR_MAX_RAM_SLOTS) { > + max_memslots = SPAPR_MAX_RAM_SLOTS; > + } > + if (machine->ram_slots > max_memslots) { > error_report("Specified number of memory slots %" > PRIu64" exceeds max supported %d", > - machine->ram_slots, SPAPR_MAX_RAM_SLOTS); > + machine->ram_slots, max_memslots); > exit(1); > } > >
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 44e401a..14cc6ae 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1816,11 +1816,21 @@ static void ppc_spapr_init(MachineState *machine) /* initialize hotplug memory address space */ if (machine->ram_size < machine->maxram_size) { ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size; + /* + * Limit the number of hotpluggable memory slots to half the number + * slots that KVM supports, leaving the other half for PCI and other + * devices. However ensure that number of slots doesn't drop below 32. + */ + int max_memslots = kvm_enabled() ? kvm_get_max_memslots() / 2 : + SPAPR_MAX_RAM_SLOTS; - if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) { + if (max_memslots < SPAPR_MAX_RAM_SLOTS) { + max_memslots = SPAPR_MAX_RAM_SLOTS; + } + if (machine->ram_slots > max_memslots) { error_report("Specified number of memory slots %" PRIu64" exceeds max supported %d", - machine->ram_slots, SPAPR_MAX_RAM_SLOTS); + machine->ram_slots, max_memslots); exit(1); }