Skip to content
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,11 @@ __pycache__/
test/e2e/**/output

test/statistics-analysis/output
*.test

# Extracted sysfs fixture directories used by pkg/sysfs, pkg/resmgr/lib/memory,
# and pkg/topology tests. These are unpacked from tarballs at test time and
# should not be tracked.
pkg/sysfs/testdata/
pkg/resmgr/lib/memory/testdata/
pkg/topology/testdata/
6 changes: 6 additions & 0 deletions cmd/plugins/balloons/policy/balloons-policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,12 @@ func (p *balloons) Start() error {
return nil
}

// Stop shuts down this policy. The balloons policy holds no resources
// to release.
func (p *balloons) Stop() error {
return nil
}

// Sync synchronizes the active policy state.
func (p *balloons) Sync(add []cache.Container, del []cache.Container) error {
irq.BlockWrites()
Expand Down
6 changes: 6 additions & 0 deletions cmd/plugins/template/policy/template-policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ func (p *policy) Start() error {
return nil
}

// Stop shuts down this policy. The template policy holds no resources
// to release.
func (p *policy) Stop() error {
return nil
}

// Reconfigure this policy.
func (p *policy) Reconfigure(newCfg any) error {
cfg, ok := newCfg.(*cfgapi.Config)
Expand Down
4 changes: 2 additions & 2 deletions cmd/plugins/topology-aware/policy/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ func TestAllocationMarshalling(t *testing.T) {
}{
{
name: "non-zero Exclusive",
data: []byte(`{"key1":{"PrettyName":"","Exclusive":"1","Part":1,"CPUType":0,"Container":"1","Pool":"testnode","MemoryPool":0,"MemType":"DRAM,PMEM,HBM","MemSize":0,"ColdStart":0}}`),
data: []byte(`{"key1":{"PrettyName":"","Exclusive":"1","Part":1,"CPUType":0,"CPUClass":"","Container":"1","Pool":"testnode","MemoryPool":0,"MemType":"DRAM,PMEM,HBM","MemSize":0,"ColdStart":0,"Irqs":null}}`),
},
{
name: "zero Exclusive",
data: []byte(`{"key1":{"PrettyName":"","Exclusive":"","Part":1,"CPUType":0,"Container":"1","Pool":"testnode","MemoryPool":0,"MemType":"DRAM,PMEM,HBM","MemSize":0,"ColdStart":0}}`),
data: []byte(`{"key1":{"PrettyName":"","Exclusive":"","Part":1,"CPUType":0,"CPUClass":"","Container":"1","Pool":"testnode","MemoryPool":0,"MemType":"DRAM,PMEM,HBM","MemSize":0,"ColdStart":0,"Irqs":null}}`),
},
}
for _, tc := range tcases {
Expand Down
123 changes: 123 additions & 0 deletions cmd/plugins/topology-aware/policy/dra.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// Copyright 2019 Intel Corporation. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package topologyaware

import (
policyapi "github.com/containers/nri-plugins/pkg/resmgr/policy"

"github.com/containers/nri-plugins/pkg/resmgr/cpuclass"
"github.com/containers/nri-plugins/pkg/resmgr/dra"
"github.com/containers/nri-plugins/pkg/utils/cpuset"
)

// buildDRAPlugin constructs p.draPlugin from the current policy

@klihub klihub Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit like we'd have put essentially the whole full blown DRA driver, running almost fully contained within the policy implementation, as an completely independent second driver, the other allocation path (via the stock resmgr NRI layer) being otherwise oblivious to it than via the shared (resmgr-level) lock. That's pretty implicit awareness observable only via more variance in the NRI-triggered code paths timings (usually not visible probably), and potentially via deadlocking which is now more easy to achieve with locking responsibility to protect against the NRI code paths having been fully delegated to the policy-contained DRA driver and it's related logic/additions.

Is there a viable alternative to this, where we'd have a delegation of responsibilities with closer resemblance to how we do it on the NRI-triggered allocation/release paths ? There we basically have the (resmgr) infrastructure/machinery take care of most boilerplate, including most (or maybe all, I don't recall now by heart) of the locking/serialization. The generic resmgr/policy interface has then functions for allocation/reallocation/release and those are used to hook in the policy-specific allocation/release/CPU,memory,etc selection logic into the generic/boilerplate (NRI) driver at the resmgr level.

So going more in that direction would mean the resmgr/policy interface gaining extra functions specific to/for the DRA-based allocation/release code paths, which then would be used to hook the policy-specific DRA decision/allocation logic into the generic/boilerplate DRA machinery, which for instance could live at the resmgr level.

I admit that such a split is easier for the traditional allocation, resulting in a rather small and simple API footprint, which is one of the key reasons why such an abstraction suggested itself as an obvious choice. This might not be the case here, or at least not such a clear cut, due to (a) DRA (device) being more versatile in this regard via device names+attributes, etc. But I think we should give it a serious and honest consideration, including thinking about the next steps of 1) exposing something more via DRA from the T-A policy, maybe for instance isolated CPU cores, P/E-cores, etc., and 2) adding DRA support to the balloons policy. Then think about what are the consequences and see what kind of compromises we'd need to make with one and the other approach, so we could take an educated guess which one looks to be less painful in the future.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps one way to drive claude towards more policy-independent DRA logic in resmgr could be asking a structure where, in addition to the topology-aware policy, the template policy would have expose all allowed CPUs in a DRA resource slice, too.

The purpose of the template policy is to provide minimal basis for anyone to implement their own policy. This would extended it to enable anyone building their own policy with DRA support, and as a side effect, would hopefully force claude to design how DRA becomes visible to multiple policies.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea! I'll try to add it to the implementation. So far the plan is to cancel this PR and continue sumbitting series of smaller and hopefully easier to review PRs. The series has been already started by #782. Will submit a second one soon.

// configuration (p.cfg, p.cpuClasses, p.cache) and the given backend
// options. Called once from Setup() when cfg.DRAEnabled() is true, and
// never from Reconfigure() (which refuses any change to DRAEnabled outright
// rather than tearing down/rebuilding p.draPlugin).
//
// A missing kube client or node name is treated as "DRA not ready yet"
// rather than a hard Setup() failure: this logs a warning and leaves
// p.draPlugin nil. Every other Backend lifecycle method already nil-checks
// p.draPlugin, so this degrades to "DRA disabled" rather than crashing.
//
// An empty (nil) p.cpuClasses is different: the plugin is still built, just
// with an empty device set (every *cpuclass.Handler method the adapter
// calls is nil-receiver-safe, see policyDRAAdapter's doc comment). Since
// buildDRAPlugin only ever runs once, leaving p.draPlugin nil here would
// prevent a later Reconfigure() that adds cpuClasses from ever enabling DRA.
//
// Genuine construction failures (CDI writer setup, dra.New's own
// dependency validation) are returned as errors, since those indicate a
// real misconfiguration rather than a timing issue.
func (p *policy) buildDRAPlugin(opts *policyapi.BackendOptions) error {
if opts.KubeClientFn == nil {
log.Warnf("dra: no KubeClientFn provided, DRA plugin not started")
return nil
}
kubeClient := opts.KubeClientFn()
if kubeClient == nil {
log.Warnf("dra: no kube client available yet, DRA plugin not started")
return nil
}
if opts.NodeName == "" {
log.Warnf("dra: node name not known yet, DRA plugin not started")
return nil
}

adapter := &policyDRAAdapter{p: p}

cdiWriter, err := dra.NewCDIWriter(DRADriverName, p.cdiDir)
if err != nil {
return policyError("failed to create DRA CDI writer: %w", err)
}

deps := dra.Deps{
KubeClient: kubeClient,
NodeName: opts.NodeName,
// ValidateClasses captures p, not a config snapshot, so it observes
// whatever p.cfg is live when called, including after Reconfigure()
// swaps it. DRASharedCounters() is the nil-safe getter — without it,
// a Reconfigure that removes the dra: section would panic on
// p.cfg.DRA.SharedCounters here.
ValidateClasses: func() error {
return cpuclass.ValidateCPUClassesForDRA(p.cfg.CPUClasses, p.cfg.DRASharedCounters())
},
// ValidateCPUsInPool mirrors the check allocateClaim performs via
// poolForCPUs at container-creation time, so a claim whose picked
// CPUs straddle more than one leaf pool (e.g. a punit spanning an
// entire package while leaf pools are NUMA or L3 nodes) is rejected
// at Prepare time instead of being persisted and always failing
// later when its container is created.
ValidateCPUsInPool: func(cpus cpuset.CPUSet) error {
_, err := p.poolForCPUs(cpus)
return err
},
DeviceLister: adapter,
ClaimAllocator: adapter,
CDIWriter: cdiWriter,
ClaimStore: dra.NewCacheClaimStore(p.cache),
ClaimUnprepare: p.unprepareDRAClaim,
WithLock: opts.WithLock,
Logger: log,
}

plugin, err := dra.New(DRADriverName, deps)
if err != nil {
return policyError("failed to create DRA plugin: %w", err)
}

p.draPlugin = plugin

return nil
}

// triggerDRARepublish enqueues a DRA ResourceSlice republication when
// PCT-based HP capacity is active. Called (deferred) from AllocateResources,
// ReleaseResources, and UpdateResources so that any change to non-DRA HP CPU
// usage (hpUsed) caused by those NRI-path allocations is reflected in the
// next ResourceSlice.
// The enqueue is non-blocking and safe to call while holding the resmgr lock;
// the actual publish runs in the DRA plugin's republisherLoop goroutine, which
// acquires the lock itself after the NRI handler releases it.
func (p *policy) triggerDRARepublish() {
if p.draPlugin == nil {
return
}
if p.cpuClasses == nil || !p.cpuClasses.PctActive() {
return
}
p.draPlugin.TriggerRepublish()
}
75 changes: 75 additions & 0 deletions cmd/plugins/topology-aware/policy/dra_adapter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// Copyright 2019 Intel Corporation. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package topologyaware

import (
resourceapi "k8s.io/api/resource/v1"

"github.com/containers/nri-plugins/pkg/resmgr/dra"
"github.com/containers/nri-plugins/pkg/utils/cpuset"
)

// DRADriverName is the DRA driver name used to publish and identify CPU
// devices for the topology-aware policy. Defined once here; referenced
// everywhere else DRA devices for this policy are published or identified.
const DRADriverName = "nri.topology-aware.cpu"

// policyDRAAdapter implements dra.ClaimAllocator and dra.DeviceLister by
// forwarding every call to the *current* p.cpuClasses at call time.
//
// A field holding *cpuclass.Handler directly (captured once, at
// construction) would go stale: initialize() sets p.cpuClasses = nil and
// installs a brand new *cpuclass.Handler on every Reconfigure. Because this
// adapter only ever holds the *policy back-pointer and dereferences
// p.cpuClasses fresh on each call, it always observes the handler that is
// live at call time.
//
// All *cpuclass.Handler methods used here are nil-receiver-safe (they check
// h == nil internally and return zero-values/errors), so no additional nil
// guard is required in the adapter for a nil p.cpuClasses.
type policyDRAAdapter struct {
p *policy
}

// Make sure policyDRAAdapter implements the interfaces the DRA plugin needs.
var (
_ dra.ClaimAllocator = &policyDRAAdapter{}
_ dra.DeviceLister = &policyDRAAdapter{}
)

// PickHpCpus routes to the current p.cpuClasses handler's PickHpCpus.
func (a *policyDRAAdapter) PickHpCpus(pkgID, punitID, n int, held cpuset.CPUSet) (cpuset.CPUSet, error) {
return a.p.cpuClasses.PickHpCpus(pkgID, punitID, n, held)
}

// ReleaseHpCpus routes to the current p.cpuClasses handler's ReleaseHpCpus.
func (a *policyDRAAdapter) ReleaseHpCpus(pkgID, punitID int, cpus cpuset.CPUSet) {
a.p.cpuClasses.ReleaseHpCpus(pkgID, punitID, cpus)
}

// AccountHpCpus routes to the current p.cpuClasses handler's AccountHpCpus.
func (a *policyDRAAdapter) AccountHpCpus(pkgID, punitID int, cpus cpuset.CPUSet) error {
return a.p.cpuClasses.AccountHpCpus(pkgID, punitID, cpus)
}

// IsHPClass routes to the current p.cpuClasses handler's IsHPClass.
func (a *policyDRAAdapter) IsHPClass(className string) bool {
return a.p.cpuClasses.IsHPClass(className)
}

// DRADevices routes to the current p.cpuClasses handler's DRADevices.
func (a *policyDRAAdapter) DRADevices(driverName string) ([]resourceapi.Device, error) {
return a.p.cpuClasses.DRADevices(driverName)
}
Loading