[libvirt] [PATCH v2 0/2] Two simple hugepage fixes
v2 of: https://www.redhat.com/archives/libvir-list/2017-June/msg00395.html diff to v1: - Moved the decision logic to a separate static function as requested in the review. - In 2/2 the function header moved to a different location as requested. Michal Privoznik (2): qemuProcessBuildDestroyHugepagesPath: create path more frequently qemuDomainAttachMemory: Crate hugepage dir if needed src/qemu/qemu_hotplug.c | 3 +++ src/qemu/qemu_process.c | 52 ++++++++++++++++++++++++++++++++++++++++++++----- src/qemu/qemu_process.h | 5 +++++ 3 files changed, 55 insertions(+), 5 deletions(-) -- 2.13.0
https://bugzilla.redhat.com/show_bug.cgi?id=1455819 Currently, the per-domain path for huge pages mmap() for qemu is created iff domain has memoryBacking and hugepages in it configured. However, this alone is not enough because there can be a DIMM module with hugepages configured too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4a66f0d5d..59cca2736 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3283,6 +3283,31 @@ qemuProcessReconnectCheckMemAliasOrderMismatch(virDomainObjPtr vm) } +static bool +qemuProcessNeedHugepagesPath(virDomainDefPtr def) +{ + const long system_pagesize = virGetSystemPageSizeKB(); + size_t i; + + if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) + return true; + + for (i = 0; i < def->mem.nhugepages; i++) { + if (def->mem.hugepages[i].size != system_pagesize) + return true; + } + + for (i = 0; i < def->nmems; i++) { + if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && + def->mems[i]->pagesize && + def->mems[i]->pagesize != system_pagesize) + return true; + } + + return false; +} + + static int qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3291,9 +3316,13 @@ qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); char *hugepagePath = NULL; size_t i; + bool shouldBuild = false; int ret = -1; - if (vm->def->mem.nhugepages) { + if (build) + shouldBuild = qemuProcessNeedHugepagesPath(vm->def); + + if (!build || shouldBuild) { for (i = 0; i < cfg->nhugetlbfs; i++) { VIR_FREE(hugepagePath); hugepagePath = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]); -- 2.13.0
On 06/13/2017 09:05 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1455819
Currently, the per-domain path for huge pages mmap() for qemu is created iff domain has memoryBacking and hugepages in it configured. However, this alone is not enough because there can be a DIMM module with hugepages configured too.
The reality is there can be multiple reasons, true? 1. Memory backing source file 2. Some page in the configured hugepages list that doesn't match the system pagesize 3. Some pagesize for a DIMM module that's requesting a different page size than the system page size
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
On 06/13/2017 04:14 PM, John Ferlan wrote:
On 06/13/2017 09:05 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1455819
Currently, the per-domain path for huge pages mmap() for qemu is created iff domain has memoryBacking and hugepages in it configured. However, this alone is not enough because there can be a DIMM module with hugepages configured too.
The reality is there can be multiple reasons, true?
1. Memory backing source file 2. Some page in the configured hugepages list that doesn't match the system pagesize 3. Some pagesize for a DIMM module that's requesting a different page size than the system page size
True.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
Pushed both. Thanks, Michal
https://bugzilla.redhat.com/show_bug.cgi?id=1455819 It may happen that a domain is started without any huge pages. However, user might try to attach a DIMM module later. DIMM backed by huge pages (why would somebody want to mix regular and huge pages is beyond me). Therefore we have to create the dir if we haven't done so far. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 3 +++ src/qemu/qemu_process.c | 25 +++++++++++++++++++------ src/qemu/qemu_process.h | 5 +++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 96f3f4579..0b8d3d80f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2258,6 +2258,9 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, priv->qemuCaps, vm->def, mem, NULL, true) < 0) goto cleanup; + if (qemuProcessBuildDestroyHugepagesPath(driver, vm, mem, true) < 0) + goto cleanup; + if (qemuDomainNamespaceSetupMemory(driver, vm, mem) < 0) goto cleanup; teardowndevice = true; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 59cca2736..85b800da3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3284,7 +3284,8 @@ qemuProcessReconnectCheckMemAliasOrderMismatch(virDomainObjPtr vm) static bool -qemuProcessNeedHugepagesPath(virDomainDefPtr def) +qemuProcessNeedHugepagesPath(virDomainDefPtr def, + virDomainMemoryDefPtr mem) { const long system_pagesize = virGetSystemPageSizeKB(); size_t i; @@ -3304,13 +3305,20 @@ qemuProcessNeedHugepagesPath(virDomainDefPtr def) return true; } + if (mem && + mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && + mem->pagesize && + mem->pagesize != system_pagesize) + return true; + return false; } -static int +int qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, virDomainObjPtr vm, + virDomainMemoryDefPtr mem, bool build) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -3320,7 +3328,7 @@ qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, int ret = -1; if (build) - shouldBuild = qemuProcessNeedHugepagesPath(vm->def); + shouldBuild = qemuProcessNeedHugepagesPath(vm->def, mem); if (!build || shouldBuild) { for (i = 0; i < cfg->nhugetlbfs; i++) { @@ -3331,6 +3339,11 @@ qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, goto cleanup; if (build) { + if (virFileExists(hugepagePath)) { + ret = 0; + goto cleanup; + } + if (virFileMakePathWithMode(hugepagePath, 0700) < 0) { virReportSystemError(errno, _("Unable to create %s"), @@ -3504,7 +3517,7 @@ qemuProcessReconnect(void *opaque) goto cleanup; } - if (qemuProcessBuildDestroyHugepagesPath(driver, obj, true) < 0) + if (qemuProcessBuildDestroyHugepagesPath(driver, obj, NULL, true) < 0) goto error; if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, @@ -5572,7 +5585,7 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, NULL) < 0) goto cleanup; - if (qemuProcessBuildDestroyHugepagesPath(driver, vm, true) < 0) + if (qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, true) < 0) goto cleanup; /* Ensure no historical cgroup for this VM is lying around bogus @@ -6259,7 +6272,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, goto endjob; } - qemuProcessBuildDestroyHugepagesPath(driver, vm, false); + qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, false); vm->def->id = -1; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index c38310b47..667d5c53d 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -38,6 +38,11 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver, virDomainPausedReason reason, qemuDomainAsyncJob asyncJob); +int qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem, + bool build); + void qemuProcessAutostartAll(virQEMUDriverPtr driver); void qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver); -- 2.13.0
On 06/13/2017 09:05 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1455819
It may happen that a domain is started without any huge pages. However, user might try to attach a DIMM module later. DIMM backed by huge pages (why would somebody want to mix regular and huge pages is beyond me). Therefore we have to create the dir if we haven't done so far.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 3 +++ src/qemu/qemu_process.c | 25 +++++++++++++++++++------ src/qemu/qemu_process.h | 5 +++++ 3 files changed, 27 insertions(+), 6 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
participants (2)
-
John Ferlan -
Michal Privoznik