New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

VM: Instance's CPU auto rebalancing/pinning #13257

Merged

tomponline merged 4 commits into canonical:main from mihalicyn:vm_instance_cpu_auto_pinning

Apr 5, 2024

Member

mihalicyn commented Apr 4, 2024 •

edited

Loading

Make our pinning balancer aware of VM instances existence.

mihalicyn force-pushed the vm_instance_cpu_auto_pinning branch from 26cbf19 to d6f8d7b Compare

April 4, 2024 15:25

tomponline changed the title ~~VM instance's CPU auto rebalancing/pinning~~ VM: instance's CPU auto rebalancing/pinning

tomponline changed the title ~~VM: instance's CPU auto rebalancing/pinning~~ VM: Instance's CPU auto rebalancing/pinning

mihalicyn force-pushed the vm_instance_cpu_auto_pinning branch from d6f8d7b to 968965b Compare

April 5, 2024 10:27

mihalicyn marked this pull request as ready for review

April 5, 2024 10:28

mihalicyn requested a review from tomponline as a code owner

April 5, 2024 10:28

Member Author

mihalicyn commented Apr 5, 2024

As usual, I'll prepare tests in lxd-ci repo after this PR is merged

mihalicyn force-pushed the vm_instance_cpu_auto_pinning branch from 968965b to 8248899 Compare

April 5, 2024 12:54

tomponline reviewed

View reviewed changes

lxd/devices.go Outdated Show resolved Hide resolved

tomponline reviewed

View reviewed changes

lxd/instance/instance_interface.go Outdated Show resolved Hide resolved

tomponline reviewed

View reviewed changes

lxd/instance/drivers/driver_qemu.go Outdated Show resolved Hide resolved

tomponline reviewed

View reviewed changes

Member

tomponline left a comment

Couple of nits, but overall looks great thanks!

mihalicyn force-pushed the vm_instance_cpu_auto_pinning branch 3 times, most recently from 0b22377 to f0e7d1c Compare

April 5, 2024 14:29

mihalicyn requested a review from tomponline

April 5, 2024 14:30

Member Author

mihalicyn commented Apr 5, 2024

Couple of nits, but overall looks great thanks!

Have updated. Thanks!

tomponline reviewed

View reviewed changes

lxd/devices.go Outdated Show resolved Hide resolved

tomponline reviewed

View reviewed changes

lxd/instance/drivers/driver_lxc.go Outdated Show resolved Hide resolved

tomponline reviewed

View reviewed changes

lxd/instance/drivers/driver_lxc.go Outdated Show resolved Hide resolved

tomponline requested changes

View reviewed changes

Member

tomponline left a comment

Please can you remove the loggers from SetAffinity and return them as wrapped errors with contextual info added to the error message.

For the affinity set we can passed in, we can include that in the caller's logger context.

mihalicyn force-pushed the vm_instance_cpu_auto_pinning branch from f0e7d1c to 1b48d64 Compare

April 5, 2024 14:47

Member Author

mihalicyn commented Apr 5, 2024

Please can you remove the loggers from SetAffinity and return them as wrapped errors with contextual info added to the error message.

Thanks! Fixed. I have left logger with with debug level only in one place where it's reasonable.

mihalicyn requested a review from tomponline

April 5, 2024 14:51

tomponline reviewed

View reviewed changes

lxd/instance/drivers/driver_lxc.go Outdated Show resolved Hide resolved

mihalicyn added 4 commits

April 5, 2024 16:58


          lxd/devices: prepare for VMs dynamic core pinning support

858858d

Refactor code a bit and make some adjustments in the comments
before supporting automatic core pinning support for VM instances
as we do for containers instances.

Signed-off-by: Alexander Mikhalitsyn <[email protected]>


          instance/driver_qemu: implement SetAffinity()

9dedf54

This VM instance method is used in the dynamic core balancer
code. Let's implement it properly.

Signed-off-by: Alexander Mikhalitsyn <[email protected]>


          instance/driver_lxc: get rid of redunant calls to cgroup.TaskSchedule…

01a97bd

…rTrigger

Calls to cgroup.TaskSchedulerTrigger can be very costly, let's avoid
calling it without real reason. We only have to call it when CPU limits
were changed for the instance.

Signed-off-by: Alexander Mikhalitsyn <[email protected]>


          instance/driver_qemu: call TaskSchedulerTrigger hook onstart/stop/update

801e031

Put calls to the cgroup.TaskSchedulerTrigger in a few critical places:
- on instance start
- on instance CPU configuration change
- on instance stop

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

mihalicyn force-pushed the vm_instance_cpu_auto_pinning branch from 1b48d64 to 801e031 Compare

April 5, 2024 14:59

mihalicyn requested a review from tomponline

April 5, 2024 15:01

tomponline approved these changes

View reviewed changes

Member

tomponline left a comment

LGTM!

tomponline merged commit 7e48a31 into canonical:main

24 of 28 checks passed

Member Author

mihalicyn commented Apr 8, 2024

Test canonical/lxd-ci#132

mihalicyn mentioned this pull request

test VM cpu auto pinning canonical/lxd-ci#132

Merged

mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request


          cpu-vm: check that automatic CPU balancing works

c03bb8a

canonical/lxd#13257

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request


          cpu-vm: check that automatic CPU balancing works

1e14f16

canonical/lxd#13257

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request


          cpu-vm: check that automatic CPU balancing works

be33b78

canonical/lxd#13257

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request


          cpu-vm: check that automatic CPU balancing works

66116ae

canonical/lxd#13257

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request


          cpu-vm: check that automatic CPU balancing works

b036d20

canonical/lxd#13257

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request


          cpu-vm: check that automatic CPU balancing works

972ef98

canonical/lxd#13257

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

mihalicyn mentioned this pull request

lxd/devices: properly handle limits.cpu for VM-type instances in devic… #13523

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet