From: Fiona Ebner <f.ebner@proxmox.com>
To: "DERUMIER, Alexandre" <Alexandre.DERUMIER@groupe-cyllene.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server 08/10] memory: add virtio-mem support
Date: Tue, 20 Dec 2022 11:26:32 +0100 [thread overview]
Message-ID: <15f5554d-3708-ac78-d2e2-1d797e55e211@proxmox.com> (raw)
In-Reply-To: <7b9306c429440304fb37601ece5ffdbad0b90e5f.camel@groupe-cyllene.com>
Am 20.12.22 um 10:50 schrieb DERUMIER, Alexandre:
>>
>>> + my $blocksize = ($MAX_MEM - $static_memory) / 32000;>>> + #round next power of 2
>>> + $blocksize = 2**(int(log($blocksize)/log(2))+1);
>>
>> What if log($blocksize)/log(2) is exactly an integer? Do we still
>> want
>> to add 1 then? If not, please use ceil(...) instead of int(...)+1.
>> Well,
>> I guess it can't happen in practice by what values are possible for
>> $MAX_MEM and $static_memory, but still.
>>
> The math seem to be good to have power of 2 as blocksize
>
> log(1)/log(2):0 blocksize:2
> log(2)/log(2):1 blocksize:4
Yes, but here you'd use 4, when 2 would also be good. If that doesn't
matter, feel free to keep it as-is; I was just wondering. It's likely
not even relevant in practice, because with the values $MAX_MEM and
$static_memory can take, I'm not sure we can even get integers for the
argument to the first log()?
> with a ceil, we don't have block begin to 2
>
> log(1)/log(2):0 blocksize:1
> log(2)/log(2):1 blocksize:2
Isn't ($MAX_MEM - $static_memory) / 32000 always strictly greater than
1? And if it could get smaller than 1, we also might have issues with
the int()+1 approach, because the result of the first log() will become
negative.
To be on the safe side we could just move the minimum check up:
my $blocksize = ($MAX_MEM - $static_memory) / 32000;
$blocksize = 2 if $blocksize < 2;
$blocksize = 2**(ceil(log($blocksize)/log(2)));
>>> + #2MB is the minimum to be aligned with THP
>>> + $blocksize = 2 if $blocksize < 2;
>>
>> Nit: $blocksize is at least 2**1 after the previous caluclation, so
>> this
>> isn't really needed.
> yes,indeed.
>
>>
>>> + my $mem_device = "virtio-mem-pci,block-
>>> size=${blocksize}M,requested-size=${node_mem}M,id=$id,memdev=mem-
>>> $id,node=$i$pciaddr";
>>> + $mem_device .= ",prealloc=on" if $conf->{hugepages};
>>
>> So prealloc=on for the device, but not prealloc=yes for the object
>> below[0]. Would you mind explaining it to me? I just found the part
>> mentioned for v7.0 here
>>
> oh, yes, sorry.
> Just look here for details:
> https://lists.gnu.org/archive/html/qemu-devel/2022-01/msg00902.html
> "A common use case for hugetlb will be using "reserve=off,prealloc=off"
> for
> the memory backend and "prealloc=on" for the virtio-mem device. This
> way, no huge pages will be reserved for the process, but we can recover
> if there are no actual huge pages when plugging memory. Libvirt is
> already prepared for this.
> "
>
Thanks! Could you please add it to the commit message?
>>> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
>>> index a18b974..0187c74 100644
>>> --- a/PVE/QemuServer/PCI.pm
>>> +++ b/PVE/QemuServer/PCI.pm
>>> @@ -249,6 +249,14 @@ sub get_pci_addr_map {
>>> 'scsihw2' => { bus => 4, addr => 1 },
>>> 'scsihw3' => { bus => 4, addr => 2 },
>>> 'scsihw4' => { bus => 4, addr => 3 },
>>> + 'virtiomem0' => { bus => 4, addr => 4 },
>>> + 'virtiomem1' => { bus => 4, addr => 5 },
>>> + 'virtiomem2' => { bus => 4, addr => 6 },
>>> + 'virtiomem3' => { bus => 4, addr => 7 },
>>> + 'virtiomem4' => { bus => 4, addr => 8 },
>>> + 'virtiomem5' => { bus => 4, addr => 9 },
>>> + 'virtiomem6' => { bus => 4, addr => 10 },
>>> + 'virtiomem7' => { bus => 4, addr => 11 },
>>
>> What if $conf->{sockets} > 8? Maybe mention the limitation in the
>> description of the 'virtio' property in the 'memory' string. Is the
>> plan
>> to just add on more virtiomem PCI devices in the future?
>>
> Well, do we really support more than 8 sockets ?
> I'm not sure than current numa implementation is working with more than
> 8 socket, and in real world, I never have seen servers with more than 8
> sockets. (I think super-expensive hardware with 16 - 32 sockets is the
> max)
> AFAIK, The main usecase of sockets/numa node, is for auto
> numabalancing, to try to map the qemu numa nodes to physical numa
> nodes.
>
> If needed, we could a dedicated pci bridge (like for virtioscsi), with
> 32 devices.
> and use something like virtiomem0 = addr=1 , virtiomem1=addr=2
>
>
Okay, makes sense. You never know what the future brings, but if it'll
take some time until we need to add more (and not that much more) I
guess it's fine.
next prev parent reply other threads:[~2022-12-20 10:26 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-09 19:27 [pve-devel] [PATCH qemu-server 00/10] rework memory hotplug + virtiomem Alexandre Derumier
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 01/10] test: add memory tests Alexandre Derumier
2022-12-16 13:38 ` Fiona Ebner
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 02/10] add memory parser Alexandre Derumier
2022-12-16 13:38 ` Fiona Ebner
2023-01-02 10:50 ` DERUMIER, Alexandre
2023-01-05 12:47 ` Fiona Ebner
2023-01-02 11:23 ` DERUMIER, Alexandre
2023-01-05 12:48 ` Fiona Ebner
[not found] ` <4ba723fb986517054761eb65f38812fac86a895b.camel@groupe-cyllene.com>
2023-01-09 14:35 ` Fiona Ebner
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 03/10] config: memory: add 'max' option Alexandre Derumier
2022-12-16 13:38 ` Fiona Ebner
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 04/10] memory: add get_static_mem Alexandre Derumier
2022-12-16 13:38 ` Fiona Ebner
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 05/10] memory: get_max_mem: use config memory max Alexandre Derumier
2022-12-16 13:39 ` Fiona Ebner
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 06/10] memory: use 64 slots && static dimm size with max is defined Alexandre Derumier
2022-12-16 13:39 ` Fiona Ebner
2022-12-19 12:05 ` DERUMIER, Alexandre
2022-12-19 12:28 ` Fiona Ebner
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 07/10] test: add memory-max tests Alexandre Derumier
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 08/10] memory: add virtio-mem support Alexandre Derumier
2022-12-16 13:42 ` Fiona Ebner
[not found] ` <7b9306c429440304fb37601ece5ffdbad0b90e5f.camel@groupe-cyllene.com>
2022-12-20 10:26 ` Fiona Ebner [this message]
2022-12-20 12:16 ` [PVE-User] " DERUMIER, Alexandre
2022-12-20 12:31 ` DERUMIER, Alexandre
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 09/10] tests: add virtio-mem tests Alexandre Derumier
2022-12-16 13:42 ` Fiona Ebner
2022-12-19 14:48 ` Thomas Lamprecht
2022-12-09 19:27 ` [pve-devel] [PATCH qemu-server 10/10] memory: fix hotplug with virtiomem && maxmem Alexandre Derumier
2022-12-16 13:42 ` Fiona Ebner
2022-12-16 13:38 ` [pve-devel] [PATCH qemu-server 00/10] rework memory hotplug + virtiomem Fiona Ebner
2022-12-19 11:31 ` DERUMIER, Alexandre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=15f5554d-3708-ac78-d2e2-1d797e55e211@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=Alexandre.DERUMIER@groupe-cyllene.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal