Skip to content

Commit d35d3e0

Browse files
committed
sys/cpu: fix CPU sampling race
* must recheck the same (elapsed-since) condition under lock - must serialize Signed-off-by: Alex Aizman <alex.aizman@gmail.com>
1 parent cb6c946 commit d35d3e0

2 files changed

Lines changed: 21 additions & 9 deletions

File tree

sys/cpu.go

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/NVIDIA/aistore/cmn/atomic"
1313
"github.com/NVIDIA/aistore/cmn/cos"
14+
"github.com/NVIDIA/aistore/cmn/debug"
1415
"github.com/NVIDIA/aistore/cmn/mono"
1516
"github.com/NVIDIA/aistore/cmn/nlog"
1617
)
@@ -76,29 +77,36 @@ type (
7677
)
7778

7879
func Refresh(now int64, periodic bool) (int64, int64, error) {
79-
if last := gcpu.last.Load(); last > 0 {
80-
elapsed := max(time.Duration(now-last), 0)
81-
if elapsed < MinIvalShort || (elapsed < minIvalLong && !periodic) {
82-
return gcpu.utilEMA.Load(), gcpu.throttled.Load(), nil
83-
}
80+
last := gcpu.last.Load()
81+
since := max(time.Duration(now-last), 0)
82+
if since < MinIvalShort || (since < minIvalLong && !periodic) {
83+
return gcpu.utilEMA.Load(), gcpu.throttled.Load(), nil
8484
}
8585

86+
gcpu.ring.mu.Lock()
87+
defer gcpu.ring.mu.Unlock()
88+
89+
last = gcpu.last.Load()
90+
since = max(time.Duration(now-last), 0)
91+
if since < MinIvalShort || (since < minIvalLong && !periodic) {
92+
return gcpu.utilEMA.Load(), gcpu.throttled.Load(), nil
93+
}
8694
cur, err := gcpu.read()
8795
if err != nil {
8896
return gcpu.utilEMA.Load(), gcpu.throttled.Load(), err
8997
}
9098
cur.ts = now
9199

92100
// read and compute current
93-
gcpu.ring.mu.Lock()
94101
rawUtil, throttled, elapsed := gcpu.ring.sample(cur)
95102
gcpu.throttled.Store(throttled)
96103
gcpu.last.Store(now)
97104

98105
// compute and store moving average
106+
// note: not differentiating the first reading (against 0 at alpha 20) from the rest -
107+
// will converge fast
99108
util := gcpu.compute(elapsed, rawUtil)
100109
gcpu.utilEMA.Store(util)
101-
gcpu.ring.mu.Unlock()
102110

103111
return util, throttled, nil
104112
}
@@ -128,11 +136,13 @@ func (r *ring) sample(cur sample) (util, throttled, elapsed int64) {
128136
r.a[r.idx] = cur
129137

130138
if prev.ts == 0 {
131-
return 0, 0, 0
139+
return 0, 0, 0 // first refresh
132140
}
133141

134142
// compute current
135143
elapsed = cur.ts - prev.ts
144+
debug.Assert(elapsed > 0)
145+
136146
if isContainerized() && cur.throttledUsec > 0 {
137147
dtUsec := max(cur.throttledUsec-prev.throttledUsec, 0)
138148
throttled = cos.DivRoundI64(dtUsec*100_000, elapsed)
@@ -167,7 +177,7 @@ func NumCPU() int { return gcpu.num }
167177
func MaxParallelism() int { return max(NumCPU(), 4) }
168178

169179
// HighLoadWM: "high-load watermark" as a percentage.
170-
// For 8 CPUs: max(100 - 100/8, 1) = 88 - between HighLoad(82) and ExtremeLoad(92).
180+
// For 8 CPUs: max(100 - 100/8, 1) = 88 - between HighLoad(85) and ExtremeLoad(95).
171181
// see also: (ExtremeLoad, HighLoad) defaults
172182
func HighLoadWM() int64 {
173183
ncpu := int64(NumCPU())

sys/init.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"runtime"
1111

1212
"github.com/NVIDIA/aistore/cmn/debug"
13+
"github.com/NVIDIA/aistore/cmn/mono"
1314
"github.com/NVIDIA/aistore/cmn/nlog"
1415
)
1516

@@ -37,6 +38,7 @@ var (
3738
// num CPUs may get adjusted by Init() below
3839
func init() {
3940
gcpu.num = runtime.NumCPU()
41+
gcpu.last.Store(mono.NanoTime() - int64(maxIval)) // non-zero and already expired
4042
}
4143

4244
func isContainerized() bool { return contDetected || contForced }

0 commit comments

Comments
 (0)