[libvirt] [PATCH 3/8] qemu: Fix memory locking limit calculation

Andrea Bolognani posted 8 patches 8 years, 10 months ago
There is a newer version of this series
[libvirt] [PATCH 3/8] qemu: Fix memory locking limit calculation
Posted by Andrea Bolognani 8 years, 10 months ago
For guests that use <memoryBacking><locked>, our only option
is to remove the memory locking limit altogether.
---
 src/qemu/qemu_domain.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2e2ba4f..01681a5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6220,10 +6220,13 @@ qemuDomainGetMemLockLimitBytes(virDomainDefPtr def)
         goto done;
     }
 
-    if (def->mem.locked) {
-        memKB = virDomainDefGetMemoryTotal(def) + 1024 * 1024;
-        goto done;
-    }
+    /* If the guest wants its memory to be locked, we need to raise the memory
+     * locking limit so that the OS will not refuse allocation requests;
+     * however, there is no reliable way for us to figure out how much memory
+     * the QEMU process will allocate for its own use, so our only way out is
+     * to remove the limit altogether. Use with extreme care */
+    if (def->mem.locked)
+        return VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
 
     if (ARCH_IS_PPC64(def->os.arch) && def->virtType == VIR_DOMAIN_VIRT_KVM) {
         unsigned long long maxMemory;
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] qemu: Fix memory locking limit calculation
Posted by Martin Kletzander 8 years, 10 months ago
On Thu, Mar 23, 2017 at 07:16:42PM +0100, Andrea Bolognani wrote:
>For guests that use <memoryBacking><locked>, our only option
>is to remove the memory locking limit altogether.
>---
> src/qemu/qemu_domain.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 2e2ba4f..01681a5 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -6220,10 +6220,13 @@ qemuDomainGetMemLockLimitBytes(virDomainDefPtr def)
>         goto done;
>     }
>
>-    if (def->mem.locked) {
>-        memKB = virDomainDefGetMemoryTotal(def) + 1024 * 1024;
>-        goto done;
>-    }
>+    /* If the guest wants its memory to be locked, we need to raise the memory
>+     * locking limit so that the OS will not refuse allocation requests;
>+     * however, there is no reliable way for us to figure out how much memory
>+     * the QEMU process will allocate for its own use, so our only way out is
>+     * to remove the limit altogether. Use with extreme care */
>+    if (def->mem.locked)
>+        return VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
>

So there is no way how one can limit the size of the memlock, other than
setting the hard limit?  Are you planning on adding new element to the
domain XML which would allow setting this number as well?

>     if (ARCH_IS_PPC64(def->os.arch) && def->virtType == VIR_DOMAIN_VIRT_KVM) {
>         unsigned long long maxMemory;
>--
>2.7.4
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] qemu: Fix memory locking limit calculation
Posted by Andrea Bolognani 8 years, 10 months ago
On Mon, 2017-03-27 at 14:24 +0200, Martin Kletzander wrote:
[...]
> > @@ -6220,10 +6220,13 @@ qemuDomainGetMemLockLimitBytes(virDomainDefPtr def)
> >         goto done;
> >     }
> > 
> > -    if (def->mem.locked) {
> > -        memKB = virDomainDefGetMemoryTotal(def) + 1024 * 1024;
> > -        goto done;
> > -    }
> > +    /* If the guest wants its memory to be locked, we need to raise the memory
> > +     * locking limit so that the OS will not refuse allocation requests;
> > +     * however, there is no reliable way for us to figure out how much memory
> > +     * the QEMU process will allocate for its own use, so our only way out is
> > +     * to remove the limit altogether. Use with extreme care */
> > +    if (def->mem.locked)
> > +        return VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> 
> So there is no way how one can limit the size of the memlock, other than
> setting the hard limit?

Correct.

> Are you planning on adding new element to the
> domain XML which would allow setting this number as well?

I do. Unless I forget about it again, of course :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] qemu: Fix memory locking limit calculation
Posted by Martin Kletzander 8 years, 10 months ago
On Mon, Mar 27, 2017 at 03:17:12PM +0200, Andrea Bolognani wrote:
>On Mon, 2017-03-27 at 14:24 +0200, Martin Kletzander wrote:
>[...]
>> > @@ -6220,10 +6220,13 @@ qemuDomainGetMemLockLimitBytes(virDomainDefPtr def)
>> >         goto done;
>> >     }
>> > 
>> > -    if (def->mem.locked) {
>> > -        memKB = virDomainDefGetMemoryTotal(def) + 1024 * 1024;
>> > -        goto done;
>> > -    }
>> > +    /* If the guest wants its memory to be locked, we need to raise the memory
>> > +     * locking limit so that the OS will not refuse allocation requests;
>> > +     * however, there is no reliable way for us to figure out how much memory
>> > +     * the QEMU process will allocate for its own use, so our only way out is
>> > +     * to remove the limit altogether. Use with extreme care */
>> > +    if (def->mem.locked)
>> > +        return VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
>> 
>> So there is no way how one can limit the size of the memlock, other than
>> setting the hard limit?
>
>Correct.
>
>> Are you planning on adding new element to the
>> domain XML which would allow setting this number as well?
>
>I do. Unless I forget about it again, of course :)
>

Well, honestly, then I feel really bad about forcing people do different
choices and changing it between releases.  I don't think anyone wants to
be checking all documentation changes every release.  But since there
are only two releases with the patch being in, it's probably okay.  But
I would *at least* be nice to mention that the lock limit is on it's way
to the XML.

>-- 
>Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] qemu: Fix memory locking limit calculation
Posted by Andrea Bolognani 8 years, 10 months ago
On Mon, 2017-03-27 at 15:42 +0200, Martin Kletzander wrote:
> > > Are you planning on adding new element to the
> > > domain XML which would allow setting this number as well?
> > 
> > I do. Unless I forget about it again, of course :)
> 
> Well, honestly, then I feel really bad about forcing people do different
> choices and changing it between releases.  I don't think anyone wants to
> be checking all documentation changes every release.  But since there
> are only two releases with the patch being in, it's probably okay.  But
> I would *at least* be nice to mention that the lock limit is on it's way
> to the XML.

I'm not going to advertise a feature before it has
materialized: this is not Hollywoo, we don't need to get
people hyped using teasers and trailers ;)

Moreover, I'm actually starting to question the utility of
such a feature altogether.

If you're using <memoryBacking><locked>, all the memory
allocated by QEMU is going to be locked, so there's no
point in setting the memory locking limit to anything but
<memtune><hard_limit>.

For all other cases our calculations should be correct, and
if it turns out that they aren't we want people to get back
to us and let us know so we can fix it for everybody rather
than work around the bug using the new setting.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] qemu: Fix memory locking limit calculation
Posted by Martin Kletzander 8 years, 10 months ago
On Mon, Mar 27, 2017 at 04:24:57PM +0200, Andrea Bolognani wrote:
>On Mon, 2017-03-27 at 15:42 +0200, Martin Kletzander wrote:
>> > > Are you planning on adding new element to the
>> > > domain XML which would allow setting this number as well?
>> > 
>> > I do. Unless I forget about it again, of course :)
>> 
>> Well, honestly, then I feel really bad about forcing people do different
>> choices and changing it between releases.  I don't think anyone wants to
>> be checking all documentation changes every release.  But since there
>> are only two releases with the patch being in, it's probably okay.  But
>> I would *at least* be nice to mention that the lock limit is on it's way
>> to the XML.
>
>I'm not going to advertise a feature before it has
>materialized: this is not Hollywoo, we don't need to get
>people hyped using teasers and trailers ;)
>
>Moreover, I'm actually starting to question the utility of
>such a feature altogether.
>
>If you're using <memoryBacking><locked>, all the memory
>allocated by QEMU is going to be locked, so there's no
>point in setting the memory locking limit to anything but
><memtune><hard_limit>.
>

Since we can only lock the whole memory, then it makes sense.

>For all other cases our calculations should be correct, and
>if it turns out that they aren't we want people to get back
>to us and let us know so we can fix it for everybody rather
>than work around the bug using the new setting.
>
>-- 
>Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list