Skip to content

Commit 031aaeb

Browse files
committed
plugins: avoid DeviceSpec and Mount copies
k8s/kubelet 1.34 updates its protocol buffer definitions and triggers the following linter errors for pluginapi DeviceSpec and Mount used in our code: copylocks: range var mount copies lock: k8s.io/kubelet/pkg/apis/deviceplugin/v1beta1.Mount contains google.golang.org/protobuf/runtime/protoimpl.MessageState contains sync.Mutex (govet In most cases we loop a slice of pluginapi.DeviceSpec/Mount and taking an instance of an element is copying. This can be avoided by accessing each element from the slice directly using a range loop index instead. Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
1 parent d9fcdf8 commit 031aaeb

File tree

4 files changed

+26
-28
lines changed

4 files changed

+26
-28
lines changed

cmd/gpu_plugin/gpu_plugin.go

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ func (dp *devicePlugin) isCompatibleDevice(name string) bool {
572572
return true
573573
}
574574

575-
func (dp *devicePlugin) devSpecForDrmFile(drmFile string) (devSpec pluginapi.DeviceSpec, devPath string, err error) {
575+
func (dp *devicePlugin) devPathForDrmFile(drmFile string) (devPath string, err error) {
576576
if dp.controlDeviceReg.MatchString(drmFile) {
577577
//Skipping possible drm control node
578578
err = os.ErrInvalid
@@ -585,13 +585,6 @@ func (dp *devicePlugin) devSpecForDrmFile(drmFile string) (devSpec pluginapi.Dev
585585
return
586586
}
587587

588-
// even querying metrics requires device to be writable
589-
devSpec = pluginapi.DeviceSpec{
590-
HostPath: devPath,
591-
ContainerPath: devPath,
592-
Permissions: "rw",
593-
}
594-
595588
return
596589
}
597590

@@ -645,16 +638,20 @@ func (dp *devicePlugin) createDeviceSpecsFromDrmFiles(cardPath string) []plugina
645638
drmFiles, _ := os.ReadDir(path.Join(cardPath, "device/drm"))
646639

647640
for _, drmFile := range drmFiles {
648-
devSpec, devPath, devSpecErr := dp.devSpecForDrmFile(drmFile.Name())
649-
if devSpecErr != nil {
641+
devPath, devPathErr := dp.devPathForDrmFile(drmFile.Name())
642+
if devPathErr != nil {
650643
continue
651644
}
652645

653646
klog.V(4).Infof("Adding %s to GPU %s", devPath, filepath.Base(cardPath))
654647

655-
specs = append(specs, devSpec)
648+
// even querying metrics requires device to be writable
649+
specs = append(specs, pluginapi.DeviceSpec{
650+
HostPath: devPath,
651+
ContainerPath: devPath,
652+
Permissions: "rw",
653+
})
656654
}
657-
658655
return specs
659656
}
660657

@@ -677,18 +674,18 @@ func (dp *devicePlugin) createMountsAndCDIDevices(cardPath, name string, devSpec
677674

678675
cedits := &spec.Devices[0].ContainerEdits
679676

680-
for _, dspec := range devSpecs {
677+
for idx := range devSpecs {
681678
cedits.DeviceNodes = append(cedits.DeviceNodes, &cdispec.DeviceNode{
682-
HostPath: dspec.HostPath,
683-
Path: dspec.ContainerPath,
684-
Permissions: dspec.Permissions,
679+
HostPath: devSpecs[idx].HostPath,
680+
Path: devSpecs[idx].ContainerPath,
681+
Permissions: devSpecs[idx].Permissions,
685682
})
686683
}
687684

688-
for _, mount := range mounts {
685+
for idx := range mounts {
689686
cedits.Mounts = append(cedits.Mounts, &cdispec.Mount{
690-
HostPath: mount.HostPath,
691-
ContainerPath: mount.ContainerPath,
687+
HostPath: mounts[idx].HostPath,
688+
ContainerPath: mounts[idx].ContainerPath,
692689
Type: "none",
693690
Options: []string{"bind", "ro"},
694691
})

cmd/gpu_plugin/gpu_plugin_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -874,13 +874,13 @@ func TestBypath(t *testing.T) {
874874
absPaths = append(absPaths, path.Join(byPath, link))
875875
}
876876

877-
for _, mount := range mounts {
878-
if !slices.Contains(absPaths, mount.ContainerPath) {
879-
t.Errorf("%s: containerpath is incorrect: %s", td.desc, mount.ContainerPath)
877+
for idx := range mounts {
878+
if !slices.Contains(absPaths, mounts[idx].ContainerPath) {
879+
t.Errorf("%s: containerpath is incorrect: %s", td.desc, mounts[idx].ContainerPath)
880880
}
881881

882-
if !slices.Contains(absPaths, mount.HostPath) {
883-
t.Errorf("%s: hostpath is incorrect: %s", td.desc, mount.HostPath)
882+
if !slices.Contains(absPaths, mounts[idx].HostPath) {
883+
t.Errorf("%s: hostpath is incorrect: %s", td.desc, mounts[idx].HostPath)
884884
}
885885
}
886886
}

cmd/npu_plugin/npu_plugin.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,14 @@ func (dp *devicePlugin) scan() (dpapi.DeviceTree, error) {
167167
}
168168

169169
// even querying metrics requires device to be writable
170-
devSpec := pluginapi.DeviceSpec{
170+
devSpec := []pluginapi.DeviceSpec{{
171171
HostPath: devPath,
172172
ContainerPath: devPath,
173173
Permissions: "rw",
174+
},
174175
}
175176

176-
deviceInfo := dpapi.NewDeviceInfo(pluginapi.Healthy, []pluginapi.DeviceSpec{devSpec}, nil, nil, nil, nil)
177+
deviceInfo := dpapi.NewDeviceInfo(pluginapi.Healthy, devSpec, nil, nil, nil, nil)
177178

178179
for i := 0; i < dp.options.sharedDevNum; i++ {
179180
devID := fmt.Sprintf("%s-%d", name, i)

pkg/deviceplugin/api.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ func NewDeviceInfo(state string, nodes []pluginapi.DeviceSpec, mounts []pluginap
6060

6161
devPaths := []string{}
6262

63-
for _, node := range nodes {
64-
devPaths = append(devPaths, node.HostPath)
63+
for idx := range nodes {
64+
devPaths = append(devPaths, nodes[idx].HostPath)
6565
}
6666

6767
topologyInfo, err := topology.GetTopologyInfo(devPaths)

0 commit comments

Comments
 (0)