From 377e39a5b739ffe533e68930ed079cf201262cc0 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Wed, 21 Apr 2021 14:11:05 -0700 Subject: [PATCH] Add support for assigning cpu group on creation In recent builds of Windows there was support added to the HCS to allow assigning a cpugroup at creation time of the VM instead of afterwards. The current approach in this repo of adding a vm after start was only a workaround as this wasn't supported at the time. The current approach isn;t ideal due to some wonky behavior on machines with multiple NUMA nodes as we can suffer performance penalties because of remote memory access on machines with > 1 node when adding a VM after start. Signed-off-by: Daniel Canter --- internal/cpugroup/cpugroup.go | 6 +++--- internal/cpugroup/cpugroup_test.go | 3 +++ internal/hcs/schema2/processor_2.go | 5 ++++- internal/uvm/cpugroups.go | 13 ++++++------- internal/uvm/create.go | 3 --- internal/uvm/create_lcow.go | 20 ++++++++++++++------ internal/uvm/create_wcow.go | 21 +++++++++++++++------ internal/uvm/start.go | 7 ------- internal/uvm/types.go | 3 --- 9 files changed, 45 insertions(+), 36 deletions(-) diff --git a/internal/cpugroup/cpugroup.go b/internal/cpugroup/cpugroup.go index 3707e82576..ab3b757efb 100644 --- a/internal/cpugroup/cpugroup.go +++ b/internal/cpugroup/cpugroup.go @@ -3,12 +3,12 @@ package cpugroup import ( "context" "encoding/json" - "errors" "fmt" "strings" "github.com/Microsoft/hcsshim/internal/hcs" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/pkg/errors" ) const NullGroupID = "00000000-0000-0000-0000-000000000000" @@ -48,7 +48,7 @@ func Create(ctx context.Context, id string, logicalProcessors []uint32) error { LogicalProcessorCount: uint32(len(logicalProcessors)), } if err := modifyCPUGroupRequest(ctx, operation, details); err != nil { - return fmt.Errorf("failed to make cpugroups CreateGroup request for details %+v with: %s", details, err) + return errors.Wrapf(err, "failed to make cpugroups CreateGroup request for details %+v", details) } return nil } @@ -65,7 +65,7 @@ func getCPUGroupConfig(ctx context.Context, id string) (*hcsschema.CpuGroupConfi } groupConfigs := &hcsschema.CpuGroupConfigurations{} if err := json.Unmarshal(cpuGroupsPresent.Properties[0], groupConfigs); err != nil { - return nil, fmt.Errorf("failed to unmarshal host cpugroups: %v", err) + return nil, errors.Wrap(err, "failed to unmarshal host cpugroups") } for _, c := range groupConfigs.CpuGroups { diff --git a/internal/cpugroup/cpugroup_test.go b/internal/cpugroup/cpugroup_test.go index fff886f6ff..07af7894f8 100644 --- a/internal/cpugroup/cpugroup_test.go +++ b/internal/cpugroup/cpugroup_test.go @@ -14,14 +14,17 @@ func TestCPUGroupCreateWithIDAndDelete(t *testing.T) { lps := []uint32{0, 1} ctx, cancel := context.WithCancel(context.Background()) defer cancel() + id, err := guid.NewV4() if err != nil { t.Fatalf("failed to create cpugroup guid with: %v", err) } + err = Create(ctx, id.String(), lps) if err != nil { t.Fatalf("failed to create cpugroup %s with: %v", id.String(), err) } + defer func() { if err := Delete(ctx, id.String()); err != nil { t.Fatalf("failed to delete cpugroup %s with: %v", id.String(), err) diff --git a/internal/hcs/schema2/processor_2.go b/internal/hcs/schema2/processor_2.go index 21fe46062b..c64f335ec7 100644 --- a/internal/hcs/schema2/processor_2.go +++ b/internal/hcs/schema2/processor_2.go @@ -3,7 +3,7 @@ * * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen) * - * API version: 2.1 + * API version: 2.5 * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git) */ @@ -17,4 +17,7 @@ type Processor2 struct { Weight int32 `json:"Weight,omitempty"` ExposeVirtualizationExtensions bool `json:"ExposeVirtualizationExtensions,omitempty"` + + // An optional object that configures the CPU Group to which a Virtual Machine is going to bind to. + CpuGroup *CpuGroup `json:"CpuGroup,omitempty"` } diff --git a/internal/uvm/cpugroups.go b/internal/uvm/cpugroups.go index 2035321bb0..17d442c88e 100644 --- a/internal/uvm/cpugroups.go +++ b/internal/uvm/cpugroups.go @@ -10,15 +10,15 @@ import ( hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" ) +// Build that assigning a cpu group on creation of a vm is supported +const cpuGroupCreateBuild = 20124 + +var errCPUGroupCreateNotSupported = fmt.Errorf("cpu group assignment on create requires a build of %d or higher", cpuGroupCreateBuild) + // ReleaseCPUGroup unsets the cpugroup from the VM func (uvm *UtilityVM) ReleaseCPUGroup(ctx context.Context) error { - groupID := uvm.cpuGroupID - if groupID == "" { - // not set, don't try to do anything - return nil - } if err := uvm.unsetCPUGroup(ctx); err != nil { - return fmt.Errorf("failed to remove VM %s from cpugroup %s", uvm.ID(), groupID) + return fmt.Errorf("failed to remove VM %s from cpugroup", uvm.ID()) } return nil } @@ -42,7 +42,6 @@ func (uvm *UtilityVM) setCPUGroup(ctx context.Context, id string) error { if err := uvm.modify(ctx, req); err != nil { return err } - uvm.cpuGroupID = id return nil } diff --git a/internal/uvm/create.go b/internal/uvm/create.go index 72d09f5dca..73355e2c06 100644 --- a/internal/uvm/create.go +++ b/internal/uvm/create.go @@ -244,9 +244,6 @@ func (uvm *UtilityVM) Close() (err error) { windows.Close(uvm.vmmemProcess) if uvm.hcsSystem != nil { - if err := uvm.ReleaseCPUGroup(ctx); err != nil { - log.G(ctx).WithError(err).Warn("failed to release VM resource") - } _ = uvm.hcsSystem.Terminate(ctx) _ = uvm.Wait() } diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index 764abb5e1e..da26278c1a 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -169,7 +169,6 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error vpciDevices: make(map[string]*VPCIDevice), physicallyBacked: !opts.AllowOvercommit, devicesPhysicallyBacked: opts.FullyPhysicallyBacked, - cpuGroupID: opts.CPUGroupID, createOpts: opts, } @@ -204,6 +203,19 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error // Align the requested memory size. memorySizeInMB := uvm.normalizeMemorySize(ctx, opts.MemorySizeInMB) + processor := &hcsschema.Processor2{ + Count: uvm.processorCount, + Limit: opts.ProcessorLimit, + Weight: opts.ProcessorWeight, + } + // We can set a cpu group for the VM at creation time in recent builds. + if opts.CPUGroupID != "" { + if osversion.Build() < cpuGroupCreateBuild { + return nil, errCPUGroupCreateNotSupported + } + processor.CpuGroup = &hcsschema.CpuGroup{Id: opts.CPUGroupID} + } + doc := &hcsschema.ComputeSystem{ Owner: uvm.owner, SchemaVersion: schemaversion.SchemaV21(), @@ -221,11 +233,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error HighMMIOBaseInMB: opts.HighMMIOBaseInMB, HighMMIOGapInMB: opts.HighMMIOGapInMB, }, - Processor: &hcsschema.Processor2{ - Count: uvm.processorCount, - Limit: opts.ProcessorLimit, - Weight: opts.ProcessorWeight, - }, + Processor: processor, }, Devices: &hcsschema.Devices{ HvSocket: &hcsschema.HvSocket2{ diff --git a/internal/uvm/create_wcow.go b/internal/uvm/create_wcow.go index d01ad4bf6b..3fc84edaca 100644 --- a/internal/uvm/create_wcow.go +++ b/internal/uvm/create_wcow.go @@ -19,6 +19,7 @@ import ( "github.com/Microsoft/hcsshim/internal/uvmfolder" "github.com/Microsoft/hcsshim/internal/wclayer" "github.com/Microsoft/hcsshim/internal/wcow" + "github.com/Microsoft/hcsshim/osversion" "github.com/containerd/ttrpc" "github.com/pkg/errors" "go.opencensus.io/trace" @@ -122,6 +123,19 @@ func prepareConfigDoc(ctx context.Context, uvm *UtilityVM, opts *OptionsWCOW, uv } } + processor := &hcsschema.Processor2{ + Count: uvm.processorCount, + Limit: opts.ProcessorLimit, + Weight: opts.ProcessorWeight, + } + // We can set a cpu group for the VM at creation time in recent builds. + if opts.CPUGroupID != "" { + if osversion.Build() < cpuGroupCreateBuild { + return nil, errCPUGroupCreateNotSupported + } + processor.CpuGroup = &hcsschema.CpuGroup{Id: opts.CPUGroupID} + } + doc := &hcsschema.ComputeSystem{ Owner: uvm.owner, SchemaVersion: schemaversion.SchemaV21(), @@ -148,11 +162,7 @@ func prepareConfigDoc(ctx context.Context, uvm *UtilityVM, opts *OptionsWCOW, uv HighMMIOBaseInMB: opts.HighMMIOBaseInMB, HighMMIOGapInMB: opts.HighMMIOGapInMB, }, - Processor: &hcsschema.Processor2{ - Count: uvm.processorCount, - Limit: opts.ProcessorLimit, - Weight: opts.ProcessorWeight, - }, + Processor: processor, }, Devices: &hcsschema.Devices{ HvSocket: &hcsschema.HvSocket2{ @@ -215,7 +225,6 @@ func CreateWCOW(ctx context.Context, opts *OptionsWCOW) (_ *UtilityVM, err error vpciDevices: make(map[string]*VPCIDevice), physicallyBacked: !opts.AllowOvercommit, devicesPhysicallyBacked: opts.FullyPhysicallyBacked, - cpuGroupID: opts.CPUGroupID, createOpts: *opts, } diff --git a/internal/uvm/start.go b/internal/uvm/start.go index b5bc4e3635..ef76b39ae1 100644 --- a/internal/uvm/start.go +++ b/internal/uvm/start.go @@ -205,13 +205,6 @@ func (uvm *UtilityVM) Start(ctx context.Context) (err error) { } }() - // assign the VM to the cpugroup specified, if any - if uvm.cpuGroupID != "" { - if err := uvm.SetCPUGroup(ctx, uvm.cpuGroupID); err != nil { - return err - } - } - // Start waiting on the utility VM. uvm.exitCh = make(chan struct{}) go func() { diff --git a/internal/uvm/types.go b/internal/uvm/types.go index 1cc53cdec7..91e44aa24d 100644 --- a/internal/uvm/types.go +++ b/internal/uvm/types.go @@ -115,9 +115,6 @@ type UtilityVM struct { // Access to this variable should be done atomically. mountCounter uint64 - // cpuGroupID is the ID of the cpugroup on the host that this UVM is assigned to - cpuGroupID string - // specifies if this UVM is created to be saved as a template IsTemplate bool