neonvm: introduce CPU sysfs state scaling flow based on the vmSpec.cpuScalingMode #1111

mikhail-sakhnov · 2024-10-14T13:37:50Z

Introduce separate CPU scaling flow based on the vmSpec.cpuScalingMode

If vmSpec.cpuScalingMode is equal to qmpScaling the logic of the
scaling is preserved as before:

Scale, if required the amount of CPUs using qmp commands.
If it is required to scale cgroups, call vm-runner /cpu_change endpoint

if vmSpec.cpuScalingMode is equal to sysfsScaling all cpu
scaling requests go directly to the vm-runner /cpu_change, which in
that configuration goes to the neonvm-daemon to reconcile required
amount of online CPUs.

Value cpuSysfsStateScaling also modifies the qemu and the kernel
arguments to enable plug all CPUs but mark as online only first one.

mikhail-sakhnov · 2024-10-15T12:51:09Z

FYI, @sharnoff
We discussed with @Omrigan that it would be great to reflect the currently used cpuScalingMode in status or annotations for better observability plus probably we want to restart pod if the cpuScalingMode was changed - that actually would allow us to perform gradual testing manually without switching default mode.

sharnoff

Partial review, haven't yet gotten to the neonvm-controller changes

neonvm-controller/cmd/main.go

Makefile

neonvm-controller/deployment.yaml

neonvm-daemon/pkg/cpuscaling/sys_fs_state_scaling.go

neonvm-runner/cmd/main.go

neonvm/apis/neonvm/v1/virtualmachine_types.go

pkg/api/types.go

Omrigan

A lot of suggestions around naming, and other.

neonvm-controller/cmd/main.go

neonvm/apis/neonvm/v1/virtualmachine_types.go

pkg/neonvm/controllers/vm_controller.go

pkg/neonvm/controllers/vm_controller_handle_cpu_scaling.go

pkg/neonvm/cpuscaling/sysfsscaling.go

neonvm-daemon/pkg/cpuscaling/sys_fs_state_scaling.go

neonvm-runner/cmd/main.go

neonvm-daemon/cmd/main.go

neonvm-runner/cmd/main.go

sharnoff

One more round. I think after this it's likely to be good to go.

pkg/neonvm/controllers/vm_controller_cpu_scaling.go

pkg/neonvm/cpuscaling/cpuscaler.go

pkg/neonvm/cpuscaling/sysfs.go

pkg/neonvm/controllers/vm_controller.go

neonvm/apis/neonvm/v1/virtualmachine_types.go

pkg/neonvm/cpuscaling/sysfs.go

pkg/neonvm/controllers/vm_controller.go

…xplicitly default the field value if it is not set Signed-off-by: Misha Sakhnov <[email protected]>

…nd mark vmSpec.Guest.CPUs.Min as online during the boot Signed-off-by: Misha Sakhnov <[email protected]>

Signed-off-by: Misha Sakhnov <[email protected]>

introduce separate CPU scaling flow based on the vmSpec.cpuScalingMode If vmSpec.cpuScalingMode is equal to `qmp_scaling` the logic of the scaling is preserved as before: - Scale, if required the amount of CPUs using qmp commands. - If it is required to scale cgroups, call vm-runner /cpu_change endpoint if vmSpec.cpuScalingMode is equal to `cpuSysfsStateScaling` all cpu scaling requests go directly to the vm-runner /cpu_change, which in that configuration goes to the neonvm-daemon to reconcile required amount of online CPUs. Value `cpuSysfsStateScaling` also modifies the qemu and the kernel arguments to enable plug all CPUs but mark as online only first one. Signed-off-by: Misha Sakhnov <[email protected]>

pass cpuScalingMode as argument to the vm-runner rename arguments, constants and functions here and there drop unused code move default cpu scaling mode to controller argument Signed-off-by: Misha Sakhnov <[email protected]>

Co-authored-by: Oleg Vasilev <[email protected]>

Signed-off-by: Misha Sakhnov <[email protected]>

Co-authored-by: Oleg Vasilev <[email protected]>

Signed-off-by: Misha Sakhnov <[email protected]>

change CpuScalingMode to be a enum type drop obsolete comments Signed-off-by: Misha Sakhnov <[email protected]>

Improve readability in pkg/neonvm/controllers/vm_controller_cpu_scaling.go by adding empty lines and comments Signed-off-by: Misha Sakhnov <[email protected]>

Change scaling logic to work with ranges instead of slices Split sys fs scaling struture to allow unit testing Add unit tests for the main reconcilation logic Signed-off-by: Misha Sakhnov <[email protected]>

Get current state through aggregated /cpu/online file instead of using per-cpu files Signed-off-by: Misha Sakhnov <[email protected]>

Co-authored-by: Em Sharnoff <[email protected]> Signed-off-by: Misha Sakhnov <[email protected]>

sharnoff

Looks great! 🚀

Left just a couple final comments

pkg/neonvm/cpuscaling/cpuscaler.go

pkg/neonvm/cpuscaling/sysfs.go

Change cpuscaling logic to work with collection of IDs Signed-off-by: Misha Sakhnov <[email protected]>

mikhail-sakhnov force-pushed the misha/cpu-scaling branch 4 times, most recently from 85608dd to dfe79f1 Compare October 14, 2024 13:47

mikhail-sakhnov changed the title ~~Misha/cpu scaling~~ neonvm-controller, neonvm-runner, neonvm-daemon: introduce CPU sysfs state scaling flow based on the vmSpec.cpuScalingMode Oct 14, 2024

mikhail-sakhnov changed the title ~~neonvm-controller, neonvm-runner, neonvm-daemon: introduce CPU sysfs state scaling flow based on the vmSpec.cpuScalingMode~~ neonvm: introduce CPU sysfs state scaling flow based on the vmSpec.cpuScalingMode Oct 14, 2024

mikhail-sakhnov force-pushed the misha/cpu-scaling branch 3 times, most recently from 94bdcba to 002f978 Compare October 14, 2024 13:57

sharnoff mentioned this pull request Oct 14, 2024

neonvm-controller: add spec.cpuScalingMode #1104

Closed

mikhail-sakhnov requested review from sharnoff, Omrigan and petuhovskiy October 14, 2024 14:05

mikhail-sakhnov mentioned this pull request Oct 14, 2024

Replace hotplug cpu by online/offline CPU #1082

Open

mikhail-sakhnov force-pushed the misha/cpu-scaling branch 3 times, most recently from 508920e to e48f902 Compare October 14, 2024 18:32

mikhail-sakhnov marked this pull request as ready for review October 14, 2024 18:36

mikhail-sakhnov mentioned this pull request Oct 14, 2024

e2e-tests: fix ps path in vm-neonvmd test #1113

Merged

sharnoff self-assigned this Oct 14, 2024

mikhail-sakhnov force-pushed the misha/cpu-scaling branch from e48f902 to 6156c36 Compare October 15, 2024 09:56

sharnoff reviewed Oct 17, 2024

View reviewed changes

mikhail-sakhnov force-pushed the misha/cpu-scaling branch 3 times, most recently from 935c4fa to 5a094ae Compare October 17, 2024 12:32

mikhail-sakhnov requested a review from sharnoff October 17, 2024 12:32

mikhail-sakhnov force-pushed the misha/cpu-scaling branch from 5a094ae to 82a4827 Compare October 17, 2024 12:45

Omrigan reviewed Oct 18, 2024

View reviewed changes

mikhail-sakhnov force-pushed the misha/cpu-scaling branch from f1af40d to 0658f05 Compare October 21, 2024 08:45

mikhail-sakhnov force-pushed the misha/cpu-scaling branch from 952fa49 to af1e76d Compare November 18, 2024 17:43

sharnoff mentioned this pull request Nov 19, 2024

Makefile: Remove trailing whitespace #1150

Merged

sharnoff reviewed Nov 19, 2024

View reviewed changes

mikhail-sakhnov force-pushed the misha/cpu-scaling branch 2 times, most recently from ffd9538 to 27299cb Compare November 19, 2024 06:36

mikhail-sakhnov assigned sharnoff and unassigned mikhail-sakhnov Nov 19, 2024

mikhail-sakhnov requested a review from sharnoff November 19, 2024 07:16

mikhail-sakhnov and others added 15 commits November 20, 2024 13:21

neonvm-controller: add spec.cpuScalingMode, change vm-controller to e…

bbf67cc

…xplicitly default the field value if it is not set Signed-off-by: Misha Sakhnov <[email protected]>

neonvmrunner,neonvmcontroller: allocate all possible CPUs to the VM a…

3bf6575

…nd mark vmSpec.Guest.CPUs.Min as online during the boot Signed-off-by: Misha Sakhnov <[email protected]>

neonvmd: expose interface to control online/offline state of the CPUs

2965457

Signed-off-by: Misha Sakhnov <[email protected]>

neonvm: apply code review fixes

18cbc72

pass cpuScalingMode as argument to the vm-runner rename arguments, constants and functions here and there drop unused code move default cpu scaling mode to controller argument Signed-off-by: Misha Sakhnov <[email protected]>

neonvm-controller: apply lint and regenerate code

310c0fd

Co-authored-by: Oleg Vasilev <[email protected]>

neonvm-controller: rename runner cpu functions

5b29c4a

Signed-off-by: Misha Sakhnov <[email protected]>

neonvm-controller: make spec.cpuScalingMode immutable if already set

2fd10b3

Signed-off-by: Misha Sakhnov <[email protected]>

Update pkg/neonvm/controllers/vm_controller_cpu_scaling.go

edf6fb2

Co-authored-by: Oleg Vasilev <[email protected]>

neonvm-runner: apply code review fixes and lint

df5036f

Signed-off-by: Misha Sakhnov <[email protected]>

neonvm: apply code review fixes

3c67618

change CpuScalingMode to be a enum type drop obsolete comments Signed-off-by: Misha Sakhnov <[email protected]>

neonvm-controller: code review fixes

842aa5d

Improve readability in pkg/neonvm/controllers/vm_controller_cpu_scaling.go by adding empty lines and comments Signed-off-by: Misha Sakhnov <[email protected]>

neonvm-daemon: apply code review fixes

8a00d46

Change scaling logic to work with ranges instead of slices Split sys fs scaling struture to allow unit testing Add unit tests for the main reconcilation logic Signed-off-by: Misha Sakhnov <[email protected]>

neonvm-daemon: apply code review fix

c746f46

Get current state through aggregated /cpu/online file instead of using per-cpu files Signed-off-by: Misha Sakhnov <[email protected]>

neonvm: apply code review fixes

e31a1e3

Co-authored-by: Em Sharnoff <[email protected]> Signed-off-by: Misha Sakhnov <[email protected]>

mikhail-sakhnov force-pushed the misha/cpu-scaling branch from c903044 to f7a2585 Compare November 20, 2024 12:21

sharnoff approved these changes Nov 20, 2024

View reviewed changes

pkg/neonvm/cpuscaling/cpuscaler.go Outdated Show resolved Hide resolved

pkg/neonvm/cpuscaling/sysfs.go Outdated Show resolved Hide resolved

mikhail-sakhnov force-pushed the misha/cpu-scaling branch from f7a2585 to d0af69f Compare November 20, 2024 15:58

neonvm: apply code review fixes

fe10eef

Change cpuscaling logic to work with collection of IDs Signed-off-by: Misha Sakhnov <[email protected]>

mikhail-sakhnov force-pushed the misha/cpu-scaling branch from d0af69f to fe10eef Compare November 20, 2024 16:03

sharnoff assigned mikhail-sakhnov and unassigned sharnoff Nov 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

neonvm: introduce CPU sysfs state scaling flow based on the vmSpec.cpuScalingMode #1111

neonvm: introduce CPU sysfs state scaling flow based on the vmSpec.cpuScalingMode #1111

mikhail-sakhnov commented Oct 14, 2024 •

edited

Loading

mikhail-sakhnov commented Oct 15, 2024

sharnoff left a comment

Omrigan left a comment

sharnoff left a comment

sharnoff left a comment

neonvm: introduce CPU sysfs state scaling flow based on the vmSpec.cpuScalingMode #1111

Are you sure you want to change the base?

neonvm: introduce CPU sysfs state scaling flow based on the vmSpec.cpuScalingMode #1111

Conversation

mikhail-sakhnov commented Oct 14, 2024 • edited Loading

mikhail-sakhnov commented Oct 15, 2024

sharnoff left a comment

Choose a reason for hiding this comment

Omrigan left a comment

Choose a reason for hiding this comment

sharnoff left a comment

Choose a reason for hiding this comment

sharnoff left a comment

Choose a reason for hiding this comment

mikhail-sakhnov commented Oct 14, 2024 •

edited

Loading