[pve-devel] [PATCH 2/2] hugepages: use hostnodes value as numanode for topology

[Wolfgang Bumiller]

On Fri, Jul 29, 2016 at 10:40:37AM +0200, Alexandre DERUMIER wrote:
> >>I don't really see the relation there, why would this restriction apply
> >>specifically with hugepages enabled? Or is it just a performance
> >>concern?
> 
> yes,it's only when hugepages is enabled.
> 
> As we reserved hugepages, I really don't known how qemu will try to split memory with
> 
> hostnodes=0;1,memory=4096
> 
> or
> 
> hostnodes=0;2-3,memory=4096
> 
> 
> I think it's random, so in this case we should reserved pages on all nodes to be sure.
> Seem to be a little be complex to manage.

Makes sense.

Looks like we still need to do some more mapping though, as the foreach
function still uses '$i % $sockets' for the numa node index.

With custom numa entries this matches neither those nor the host nodes,
especially with hostnodes defined.

I think we can do it this way:

* in foreach_dimm: Count existing $conf->{numaX} and if any are
  defined use that count instead of $sockets.
* go through foreach* uses and make sure numa nodes are mapped
  accordingly:
  - qemu_memory_hotplug(): foreach_dimm's passed $numanode parameter is
    used in two ways:

    |     my $hugepages_topology->{$hugepages_size}->{$numanode} = hugepages_nr($dimm_size, $hugepages_size);

    This needs to be mapped to $conf->{numa$numanode}'s hostnode.

    |     eval { PVE::QemuServer::vm_mon_cmd($vmid, "device_add", driver => "pc-dimm", id => "$name", memdev => "mem-$name", node => $numanode) };

    This should be fine once foreach_dimm() takes numaX entries into
    account as it refers to the guest's node index AFAIK.

  - config(): Should be fine, refers to guest node index.

  - hugepages_topology(): Needs to be mapped to hostnode.

If the above makes sense to you I can prepare a patch for you to review?

> 
> 
> ----- Mail original -----
> De: "Wolfgang Bumiller" <w.bumiller at proxmox.com>
> À: "aderumier" <aderumier at odiso.com>
> Cc: "pve-devel" <pve-devel at pve.proxmox.com>
> Envoyé: Vendredi 29 Juillet 2016 09:48:04
> Objet: Re: [pve-devel] [PATCH 2/2] hugepages: use hostnodes value as numanode for topology
> 
> On Fri, Jul 29, 2016 at 05:17:34AM +0200, Alexandre Derumier wrote: 
> > also disallow more than 1 hostnode value 
> 
> I don't really see the relation there, why would this restriction apply 
> specifically with hugepages enabled? Or is it just a performance 
> concern? 
> 
> Patch seems fine otherwise. 
> 
> > 
> > Signed-off-by: Alexandre Derumier <aderumier at odiso.com> 
> > --- 
> > PVE/QemuServer/Memory.pm | 35 +++++++++++++++++++++++------------ 
> > 1 file changed, 23 insertions(+), 12 deletions(-) 
> > 
> > diff --git a/PVE/QemuServer/Memory.pm b/PVE/QemuServer/Memory.pm 
> > index 58c7217..37f285a 100644 
> > --- a/PVE/QemuServer/Memory.pm 
> > +++ b/PVE/QemuServer/Memory.pm 
> > @@ -230,17 +230,8 @@ sub config { 
> > # hostnodes 
> > my $hostnodelists = $numa->{hostnodes}; 
> > if (defined($hostnodelists)) { 
> > - my $hostnodes; 
> > - foreach my $hostnoderange (@$hostnodelists) { 
> > - my ($start, $end) = @$hostnoderange; 
> > - $hostnodes .= ',' if $hostnodes; 
> > - $hostnodes .= $start; 
> > - $hostnodes .= "-$end" if defined($end); 
> > - $end //= $start; 
> > - for (my $i = $start; $i <= $end; ++$i ) { 
> > - die "host NUMA node$i doesn't exist\n" if ! -d "/sys/devices/system/node/node$i/"; 
> > - } 
> > - } 
> > + 
> > + my $hostnodes = print_numa_hostnodes($hostnodelists); 
> > 
> > # policy 
> > my $policy = $numa->{policy}; 
> > @@ -311,6 +302,23 @@ sub print_mem_object { 
> > 
> > } 
> > 
> > +sub print_numa_hostnodes { 
> > + my ($hostnodelists) = @_; 
> > + 
> > + my $hostnodes; 
> > + foreach my $hostnoderange (@$hostnodelists) { 
> > + my ($start, $end) = @$hostnoderange; 
> > + $hostnodes .= ',' if $hostnodes; 
> > + $hostnodes .= $start; 
> > + $hostnodes .= "-$end" if defined($end); 
> > + $end //= $start; 
> > + for (my $i = $start; $i <= $end; ++$i ) { 
> > + die "host NUMA node$i doesn't exist\n" if ! -d "/sys/devices/system/node/node$i/"; 
> > + } 
> > + } 
> > + return $hostnodes; 
> > +} 
> > + 
> > sub hugepages_mount { 
> > 
> > my $mountdata = PVE::ProcFSTools::parse_proc_mounts(); 
> > @@ -406,9 +414,12 @@ sub hugepages_topology { 
> > 
> > $numa_custom_topology = 1; 
> > my $numa_memory = $numa->{memory}; 
> > + my $hostnodelists = $numa->{hostnodes}; 
> > + my $hostnodes = print_numa_hostnodes($hostnodelists); 
> > 
> > + die "more than 1 hostnode value in numa node is not supported when hugepages are enabled" if $hostnodes !~ m/^(\d)$/; 
> > my $hugepages_size = hugepages_size($conf, $numa_memory); 
> > - $hugepages_topology->{$hugepages_size}->{$i} += hugepages_nr($numa_memory, $hugepages_size); 
> > + $hugepages_topology->{$hugepages_size}->{$hostnodes} += hugepages_nr($numa_memory, $hugepages_size); 
> > 
> > } 
> > 
> > -- 
> > 2.1.4 
> > 
> > _______________________________________________ 
> > pve-devel mailing list 
> > pve-devel at pve.proxmox.com 
> > http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
> > 
> 
>