[libvirt] [PATCH 0/3] Fix memlock limit during hotplug of mdev devices

Eric Farman posted 3 patches 4 years, 7 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190903200948.75126-1-farman@linux.ibm.com
src/qemu/qemu_domain.c  | 30 ++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h  |  2 ++
src/qemu/qemu_hotplug.c | 22 ++++++++++++----------
3 files changed, 44 insertions(+), 10 deletions(-)
[libvirt] [PATCH 0/3] Fix memlock limit during hotplug of mdev devices
Posted by Eric Farman 4 years, 7 months ago
The routine qemuDomainGetMemLockLimitBytes() has a couple tests
to determine what the maximum limit of locked memory should be.
If I start a domain without any vfio stuff, /proc/$PID/limits says
the limit is 64KiB.  If I start a guest with a vfio-ccw hostdev,
the limit is now $GUEST_MEMORY + 1GiB.  It doesn't matter if I
have one hostdev or a thousand; it's always 1GiB more than the
configured amount of guest memory.

If I start a guest without any vfio devices, and hotplug that same
vfio-ccw hostdev, the limit remains at 64KiB and I start getting
I/O errors in the guest.  This makes sense, since some of the I/O
chains are long enough to exceed that page limit, and the host
starts throwing errors.

There is already code that adjusts this limit during the hotplug
of vfio-pci devices, so patch 1 refactors that code so that it
can be re-used on the mdev hotplug path in patch 3.

Patch 2, meanwhile, adds some cleanup that I think is missing in
the vfio-pci path, based on my read of the mdev path.

  $ grep locked /proc/83543/limits 
  Max locked memory         65536                65536                bytes     
  $ virsh attach-device guest scratch-ca8b.xml 
  Device attached successfully

  $ grep locked /proc/83543/limits 
  Max locked memory         3221225472           3221225472           bytes     


Eric Farman (3):
  qemu: Refactor the max memlock routine
  qemu: Reset the maximum locked memory on hotplug fail
  qemu: Adjust max memlock on mdev hotplug

 src/qemu/qemu_domain.c  | 30 ++++++++++++++++++++++++++++++
 src/qemu/qemu_domain.h  |  2 ++
 src/qemu/qemu_hotplug.c | 22 ++++++++++++----------
 3 files changed, 44 insertions(+), 10 deletions(-)

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix memlock limit during hotplug of mdev devices
Posted by Daniel Henrique Barboza 4 years, 7 months ago
Hi,


I've thought about the issue you're fixing. Have you considered/tried
to handle the MemLockLimit upper in the call hierarchy with your new
qemuDomainAdjustMaxMemLockHostdev() function? Both
qemuDomainAttachMediatedDevice() and qemuDomainAttachHostPCIDevice()
are called from qemuDomainAttachHostDevice(). In theory, a call to
qemuDomainAdjustMaxMemLockHostdev() at the start of AttachHostDevice()
would cover all hostdev types of the function. Then you can undo the
memLock in the 'error' label there if something goes wrong.

It is possible to even argue about the be possibility of calling
qemuDomainAdjustMaxMemLock() at the start of the generic hotplug
function, qemuDomainAttachDeviceLive(), using the same principle -
temporarily add the device in the vmdef, define the new mem limit
assuming that everything will be OK, undo the adjustment if ret != 0.
This would cover the case of memory hotplug, which needs RLIMIT
changes as well and uses the same qemuDomainAdjustMaxMemLock()
function to do it.


This is more of a future work though. For now I believe it's appropriate
to fix vfio-ccw hotplug ASAP.


Thanks,


DHB



On 9/3/19 5:09 PM, Eric Farman wrote:
> The routine qemuDomainGetMemLockLimitBytes() has a couple tests
> to determine what the maximum limit of locked memory should be.
> If I start a domain without any vfio stuff, /proc/$PID/limits says
> the limit is 64KiB.  If I start a guest with a vfio-ccw hostdev,
> the limit is now $GUEST_MEMORY + 1GiB.  It doesn't matter if I
> have one hostdev or a thousand; it's always 1GiB more than the
> configured amount of guest memory.
>
> If I start a guest without any vfio devices, and hotplug that same
> vfio-ccw hostdev, the limit remains at 64KiB and I start getting
> I/O errors in the guest.  This makes sense, since some of the I/O
> chains are long enough to exceed that page limit, and the host
> starts throwing errors.
>
> There is already code that adjusts this limit during the hotplug
> of vfio-pci devices, so patch 1 refactors that code so that it
> can be re-used on the mdev hotplug path in patch 3.
>
> Patch 2, meanwhile, adds some cleanup that I think is missing in
> the vfio-pci path, based on my read of the mdev path.
>
>    $ grep locked /proc/83543/limits
>    Max locked memory         65536                65536                bytes
>    $ virsh attach-device guest scratch-ca8b.xml
>    Device attached successfully
>
>    $ grep locked /proc/83543/limits
>    Max locked memory         3221225472           3221225472           bytes
>
>
> Eric Farman (3):
>    qemu: Refactor the max memlock routine
>    qemu: Reset the maximum locked memory on hotplug fail
>    qemu: Adjust max memlock on mdev hotplug
>
>   src/qemu/qemu_domain.c  | 30 ++++++++++++++++++++++++++++++
>   src/qemu/qemu_domain.h  |  2 ++
>   src/qemu/qemu_hotplug.c | 22 ++++++++++++----------
>   3 files changed, 44 insertions(+), 10 deletions(-)
>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix memlock limit during hotplug of mdev devices
Posted by Eric Farman 4 years, 7 months ago

On 9/6/19 10:29 AM, Daniel Henrique Barboza wrote:
> Hi,
> 
> 
> I've thought about the issue you're fixing. Have you considered/tried
> to handle the MemLockLimit upper in the call hierarchy with your new
> qemuDomainAdjustMaxMemLockHostdev() function? Both
> qemuDomainAttachMediatedDevice() and qemuDomainAttachHostPCIDevice()
> are called from qemuDomainAttachHostDevice(). In theory, a call to

That's a good point.  I did start going down that path, but that was
before I created qemuDomainAdjustMaxMemLockHostdev() and didn't like
the way things were turning out.  I will revisit that, since it would
certainly be better to have them adjusted in one place, rather than in
the different qemuDomainAttach* routines that are affected now (or may
be in the future).

> qemuDomainAdjustMaxMemLockHostdev() at the start of AttachHostDevice()
> would cover all hostdev types of the function. Then you can undo the
> memLock in the 'error' label there if something goes wrong.
> 
> It is possible to even argue about the be possibility of calling
> qemuDomainAdjustMaxMemLock() at the start of the generic hotplug
> function, qemuDomainAttachDeviceLive(), using the same principle -
> temporarily add the device in the vmdef, define the new mem limit
> assuming that everything will be OK, undo the adjustment if ret != 0.
> This would cover the case of memory hotplug, which needs RLIMIT
> changes as well and uses the same qemuDomainAdjustMaxMemLock()
> function to do it.>

That would be excellent.

> 
> This is more of a future work though. For now I believe it's appropriate
> to fix vfio-ccw hotplug ASAP.
> 

I agree.  I have a few other things at the moment, but will look into
these ideas after I get them sorted out.

Thanks for the reviews, and the suggestions!

 - Eric

> 
> Thanks,
> 
> 
> DHB
> 
> 
> 
> On 9/3/19 5:09 PM, Eric Farman wrote:
>> The routine qemuDomainGetMemLockLimitBytes() has a couple tests
>> to determine what the maximum limit of locked memory should be.
>> If I start a domain without any vfio stuff, /proc/$PID/limits says
>> the limit is 64KiB.  If I start a guest with a vfio-ccw hostdev,
>> the limit is now $GUEST_MEMORY + 1GiB.  It doesn't matter if I
>> have one hostdev or a thousand; it's always 1GiB more than the
>> configured amount of guest memory.
>>
>> If I start a guest without any vfio devices, and hotplug that same
>> vfio-ccw hostdev, the limit remains at 64KiB and I start getting
>> I/O errors in the guest.  This makes sense, since some of the I/O
>> chains are long enough to exceed that page limit, and the host
>> starts throwing errors.
>>
>> There is already code that adjusts this limit during the hotplug
>> of vfio-pci devices, so patch 1 refactors that code so that it
>> can be re-used on the mdev hotplug path in patch 3.
>>
>> Patch 2, meanwhile, adds some cleanup that I think is missing in
>> the vfio-pci path, based on my read of the mdev path.
>>
>>    $ grep locked /proc/83543/limits
>>    Max locked memory         65536                65536               
>> bytes
>>    $ virsh attach-device guest scratch-ca8b.xml
>>    Device attached successfully
>>
>>    $ grep locked /proc/83543/limits
>>    Max locked memory         3221225472           3221225472          
>> bytes
>>
>>
>> Eric Farman (3):
>>    qemu: Refactor the max memlock routine
>>    qemu: Reset the maximum locked memory on hotplug fail
>>    qemu: Adjust max memlock on mdev hotplug
>>
>>   src/qemu/qemu_domain.c  | 30 ++++++++++++++++++++++++++++++
>>   src/qemu/qemu_domain.h  |  2 ++
>>   src/qemu/qemu_hotplug.c | 22 ++++++++++++----------
>>   3 files changed, 44 insertions(+), 10 deletions(-)
>>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list