Re: [Qemu-devel] [RFC PATCH v0 0/2] Increase max memslots to 512 for PowerPC

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4489 bytes --]

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);
>          }
>  
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]