On Tue, Mar 22, 2022 at 04:05:14PM +0100, Michal Privoznik wrote:
Since its commit v5.0.0-rc0~75^2~1^2~3 QEMU is capable of
Anything wrong with the old "commit ffac16fab33b" ?
specifying number of threads used to allocate memory. While it
defaults to the number of vCPUs, users might want to use a
different value (especially for humongous guests with gigantic
pages).
In general, on QEMU cmd line level it is possible to use
different number of threads per each memory-backend-* object, in
practical terms it's not useful. Therefore, use <memoryBacking/>
to set guest wide value and let all memory devices 'inherit' it,
silently. IOW, don't introduce per device knob because that would
only complicate things for a little or no benefit.
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
docs/formatdomain.rst | 6 ++++--
docs/schemas/domaincommon.rng | 19 +++++++++++++------
src/conf/domain_conf.c | 15 ++++++++++++++-
src/conf/domain_conf.h | 1 +
tests/qemuxml2argvdata/memfd-memory-numa.xml | 2 +-
5 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 9b1b69bb4d..8e25474db0 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1004,7 +1004,7 @@ Memory Backing
<locked/>
<source type="file|anonymous|memfd"/>
<access mode="shared|private"/>
- <allocation mode="immediate|ondemand"/>
+ <allocation mode="immediate|ondemand" threads='8'/>
<discard/>
</memoryBacking>
...
@@ -1054,7 +1054,9 @@ influence how virtual memory pages are backed by host pages.
"private". This can be overridden per numa node by ``memAccess``.
``allocation``
Using the ``mode`` attribute, specify when to allocate the memory by
- supplying either "immediate" or "ondemand".
+ supplying either "immediate" or "ondemand". :since:`Since 8.2.0` this
+ attribute is optional among with ``threads`` attribute, that sets the number
+ of threads that hypervisor uses to allocate memory.
This is weird to read. Just say "Using the optional mode attribute" and refer to threads as optional too. If anyone wants to use just the allocation threads and leave out the mode they have to be on 8.2.0 anyway, so no need to complicate things.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e0dfc9e45f..2414a806d0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18914,6 +18914,13 @@ virDomainDefParseMemory(virDomainDef *def,
VIR_FREE(tmp);
}
+ if (virXPathUInt("string(./memoryBacking/allocation/@threads)",
+ ctxt, &def->mem.allocation_threads) == -2) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Failed to parse memory allocation threads"));
+ return -1;
+ }
+
if (virXPathNode("./memoryBacking/hugepages", ctxt)) {
/* hugepages will be used */
if ((n = virXPathNodeSet("./memoryBacking/hugepages/page", ctxt, &nodes)) < 0) {
@@ -27464,6 +27471,7 @@ virDomainMemorybackingFormat(virBuffer *buf,
const virDomainMemtune *mem)
{
g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+ g_auto(virBuffer) allocAttrBuf = VIR_BUFFER_INITIALIZER;
if (mem->nhugepages)
virDomainHugepagesFormat(&childBuf, mem->hugepages, mem->nhugepages);
@@ -27478,8 +27486,13 @@ virDomainMemorybackingFormat(virBuffer *buf,
virBufferAsprintf(&childBuf, "<access mode='%s'/>\n",
virDomainMemoryAccessTypeToString(mem->access));
if (mem->allocation)
- virBufferAsprintf(&childBuf, "<allocation mode='%s'/>\n",
+ virBufferAsprintf(&allocAttrBuf, " mode='%s'",
virDomainMemoryAllocationTypeToString(mem->allocation));
+ if (mem->allocation_threads)
Here you check if (mem->allocation_threads), but in 3/4 you check if (allocation_threads > 0), which is a bit inconsistent. I prefer the former although I know we had some disputes with this, so pick whatever one and make it consistent.
Attachment:
signature.asc
Description: PGP signature
