Skip to content
This repository was archived by the owner on Sep 9, 2024. It is now read-only.

Commit 17f098a

Browse files
Introduce pool level mutex for thread safety and corresponding tests (#14)
* introduced pool level mutex for thread safety and correspoding tests * corrected feature name typo * added a new integration test for concurrent cpu moves * improved thread safety with cpu.consolidate() * Address race-condition issue in new Checkmarx scan This PR will make use of channels to pass go routines results back to doConcurrentMoveCPUSetProfile * Bumping go to 1.21 to keep library in sync with manager --------- Co-authored-by: Lukasz Danilczuk <lukasz.danilczuk@intel.com>
1 parent eeff38e commit 17f098a

15 files changed

+346
-69
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/intel/power-optimization-library
22

3-
go 1.20
3+
go 1.21
44

55
require (
66
github.com/go-logr/logr v1.2.4

pkg/power/c_states.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func initCStates() featureStatus {
4343
driver = strings.TrimSuffix(driver, "\n")
4444
feature.driver = driver
4545
if err != nil {
46-
feature.err = fmt.Errorf("failed to determine driver")
46+
feature.err = fmt.Errorf("failed to determine driver: %w", err)
4747
return feature
4848
}
4949
if !isSupportedCStatesDriver(driver) {

pkg/power/cpu.go

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ type Cpu interface {
1616
getPool() Pool
1717
doSetPool(pool Pool) error
1818
consolidate() error
19+
consolidate_unsafe() error
1920

2021
// C-States stuff
2122
SetCStates(cStates CStates) error
@@ -26,7 +27,7 @@ type Cpu interface {
2627

2728
type cpuImpl struct {
2829
id uint
29-
mutex *sync.Mutex
30+
mutex sync.Locker
3031
pool Pool
3132
core *cpuCore
3233
// C-States properties
@@ -44,6 +45,11 @@ func newCpu(coreID uint, core *cpuCore) (Cpu, error) {
4445
}
4546

4647
func (cpu *cpuImpl) consolidate() error {
48+
cpu.mutex.Lock()
49+
defer cpu.mutex.Unlock()
50+
return cpu.consolidate_unsafe()
51+
}
52+
func (cpu *cpuImpl) consolidate_unsafe() error {
4753
if err := cpu.updateFrequencies(); err != nil {
4854
return err
4955
}
@@ -53,6 +59,9 @@ func (cpu *cpuImpl) consolidate() error {
5359
return nil
5460
}
5561

62+
63+
64+
5665
// SetPool moves current core to a specified target pool
5766
// allowed movements are reservedPoolType <-> sharedPoolType and sharedPoolType <-> any exclusive pool
5867
func (cpu *cpuImpl) SetPool(targetPool Pool) error {
@@ -77,6 +86,9 @@ func (cpu *cpuImpl) SetPool(targetPool Pool) error {
7786
}
7887

7988
log.Info("Set pool", "cpu", cpu.id, "source pool", cpu.pool.Name(), "target pool", targetPool.Name())
89+
cpu.mutex.Lock()
90+
defer cpu.mutex.Unlock()
91+
8092
if cpu.pool == targetPool { // case 0,1,5
8193
return nil
8294
}
@@ -102,17 +114,19 @@ func (cpu *cpuImpl) SetPool(targetPool Pool) error {
102114
}
103115

104116
func (cpu *cpuImpl) doSetPool(pool Pool) error {
105-
log.V(4).Info("mutex locking cpu", "coreID", cpu.id)
106-
cpu.mutex.Lock()
107-
108-
defer func() {
109-
log.V(4).Info("mutex unlocking cpu", "coreID", cpu.id)
110-
cpu.mutex.Unlock()
111-
}()
117+
cpu.pool.poolMutex().Lock()
118+
pool.poolMutex().Lock()
119+
log.V(4).Info("acquired mutexes", "source", cpu.pool.Name(), "target", pool.Name(), "cpu")
112120

113121
origPool := cpu.pool
114122
cpu.pool = pool
115123

124+
defer func() {
125+
log.V(4).Info("releasing mutexes", "source", origPool.Name(), "target", pool.Name())
126+
origPool.poolMutex().Unlock()
127+
pool.poolMutex().Unlock()
128+
}()
129+
116130
origPoolCpus := origPool.Cpus()
117131
log.V(4).Info("removing cpu from pool", "pool", origPool.Name(), "coreID", cpu.id)
118132
if err := origPoolCpus.remove(cpu); err != nil {
@@ -121,7 +135,7 @@ func (cpu *cpuImpl) doSetPool(pool Pool) error {
121135
}
122136

123137
log.V(4).Info("starting consolidation of cpu", "coreID", cpu.id)
124-
if err := cpu.consolidate(); err != nil {
138+
if err := cpu.consolidate_unsafe(); err != nil {
125139
cpu.pool = origPool
126140
origPoolCpus.add(cpu)
127141
return err

0 commit comments

Comments
 (0)