fix(drm): give the DRM master retry a fixed deadline
The retry delay was re-armed from scratch on every pass of the output loop, so any other wakeup (watchdog pings, cursor moves, reports) pushed it back: past about 0.6 s of backoff it never fired and the screen stayed frozen, the bug it was meant to fix. The retry now keeps a deadline. A seat disable ends a pending retry so no modeset follows the plane reset. A protected (locked) output whose modeset is refused for lack of master is retried too. Running out of attempts is logged. Tests cover retries under a stream of pings and the seat disable.
- commit
- bfea45c768915936d9f776ea82c97e723215f72c
- author
- brice · · 2026-10-10 14:44 UTC
- committer
- brice · · 2026-10-10 15:39 UTC
- parent
- 88c31e6
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +170 -71
M internal/adapters/drm/output.go +20 -6
| @@ -1119,6 +1119,12 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||
| 1119 | 1119 | reacquireTimer.Stop() | |
| 1120 | 1120 | defer reacquireTimer.Stop() | |
| 1121 | 1121 | var reacquire masterRetry | |
| 1122 | + | // refuse records an enable refused for lack of DRM master. | |
| 1123 | + | refuse := func(err error) { | |
| 1124 | + | if reacquire.refused(err, enabled, time.Now()) { | |
| 1125 | + | o.log.Warn().Err(err).Str("connector", o.conn.name).Int("attempts", masterRetryLimit).Msg("DRM master not back; output stays off until the next seat change") | |
| 1126 | + | } | |
| 1127 | + | } | |
| 1122 | 1128 | // setActive applies a seat change. skip reports that the loop must | |
| 1123 | 1129 | // go on without touching the frame state (refused or protected). | |
| 1124 | 1130 | setActive := func(on bool) (skip bool, err error) { | |
| @@ -1131,11 +1137,12 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||
| 1131 | 1137 | if !o.commitFailed(err, &enabled) { | |
| 1132 | 1138 | return true, err | |
| 1133 | 1139 | } | |
| 1134 | - | reacquire.refused(err, &enabled) | |
| 1140 | + | refuse(err) | |
| 1135 | 1141 | return true, nil | |
| 1136 | 1142 | } | |
| 1137 | 1143 | } | |
| 1138 | - | reacquire.done() | |
| 1144 | + | // With images, the protected modeset runs at the top of the | |
| 1145 | + | // loop (prepareSecurity): it ends or reschedules the retry. | |
| 1139 | 1146 | return true, nil | |
| 1140 | 1147 | } | |
| 1141 | 1148 | // Modeset on every enable: a fast disable+enable can coalesce to one true. | |
| @@ -1151,7 +1158,7 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||
| 1151 | 1158 | if !o.commitFailed(err, &enabled) { | |
| 1152 | 1159 | return true, err | |
| 1153 | 1160 | } | |
| 1154 | - | reacquire.refused(err, &enabled) | |
| 1161 | + | refuse(err) | |
| 1155 | 1162 | return true, nil // epoch retry or wait disabled for master reacquisition | |
| 1156 | 1163 | } | |
| 1157 | 1164 | reacquire.done() | |
| @@ -1186,6 +1193,11 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||
| 1186 | 1193 | if !o.commitFailed(err, &enabled) { | |
| 1187 | 1194 | return err | |
| 1188 | 1195 | } | |
| 1196 | + | // A lock screen refused for lack of DRM master after a VT | |
| 1197 | + | // switch is retried like an ordinary enable. | |
| 1198 | + | refuse(err) | |
| 1199 | + | } else { | |
| 1200 | + | reacquire.done() | |
| 1189 | 1201 | } | |
| 1190 | 1202 | // Recheck immediately: a takeover during the clear needs its own | |
| 1191 | 1203 | // transaction, not another scene or wake to make progress. | |
| @@ -1273,7 +1285,7 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||
| 1273 | 1285 | } | |
| 1274 | 1286 | // An enable refused for lack of DRM master is retried on time. | |
| 1275 | 1287 | var reacquireDue <-chan time.Time | |
| 1276 | - | if d, ok := reacquire.next(); ok { | |
| 1288 | + | if d, ok := reacquire.next(time.Now()); ok { | |
| 1277 | 1289 | reacquireTimer.Reset(d) | |
| 1278 | 1290 | reacquireDue = reacquireTimer.C | |
| 1279 | 1291 | } else { | |
| @@ -1342,7 +1354,9 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||
| 1342 | 1354 | case ack := <-o.seatDisable: | |
| 1343 | 1355 | o.bypassForSeatDisable() | |
| 1344 | 1356 | // The seat is about to be disabled: no frame may select the | |
| 1345 | - | // pipeline again before the next enable (a modeset). | |
| 1357 | + | // pipeline again before the next enable (a modeset), not even a | |
| 1358 | + | // retried one. | |
| 1359 | + | reacquire.done() | |
| 1346 | 1360 | enabled = false | |
| 1347 | 1361 | ack <- struct{}{} | |
| 1348 | 1362 | continue | |
| @@ -1359,7 +1373,7 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||
| 1359 | 1373 | case <-reacquireDue: | |
| 1360 | 1374 | // The seat reported active before the kernel gave DRM master | |
| 1361 | 1375 | // back: the enable modeset was refused. Try it again. | |
| 1362 | - | reacquire.attempts++ | |
| 1376 | + | reacquire.fire() | |
| 1363 | 1377 | skip, err := setActive(true) | |
| 1364 | 1378 | if err != nil { | |
| 1365 | 1379 | return err | |
| @@ -1119,6 +1119,12 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||||
| 1119 | reacquireTimer.Stop() | 1119 | reacquireTimer.Stop() | ||
| 1120 | defer reacquireTimer.Stop() | 1120 | defer reacquireTimer.Stop() | ||
| 1121 | var reacquire masterRetry | 1121 | var reacquire masterRetry | ||
| 1122 | + | // refuse records an enable refused for lack of DRM master. | |||
| 1123 | + | refuse := func(err error) { | |||
| 1124 | + | if reacquire.refused(err, enabled, time.Now()) { | |||
| 1125 | + | o.log.Warn().Err(err).Str("connector", o.conn.name).Int("attempts", masterRetryLimit).Msg("DRM master not back; output stays off until the next seat change") | |||
| 1126 | + | } | |||
| 1127 | + | } | |||
| 1122 | // setActive applies a seat change. skip reports that the loop must | 1128 | // setActive applies a seat change. skip reports that the loop must | ||
| 1123 | // go on without touching the frame state (refused or protected). | 1129 | // go on without touching the frame state (refused or protected). | ||
| 1124 | setActive := func(on bool) (skip bool, err error) { | 1130 | setActive := func(on bool) (skip bool, err error) { | ||
| @@ -1131,11 +1137,12 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||||
| 1131 | if !o.commitFailed(err, &enabled) { | 1137 | if !o.commitFailed(err, &enabled) { | ||
| 1132 | return true, err | 1138 | return true, err | ||
| 1133 | } | 1139 | } | ||
| 1134 | - | reacquire.refused(err, &enabled) | 1140 | + | refuse(err) |
| 1135 | return true, nil | 1141 | return true, nil | ||
| 1136 | } | 1142 | } | ||
| 1137 | } | 1143 | } | ||
| 1138 | - | reacquire.done() | 1144 | + | // With images, the protected modeset runs at the top of the |
| 1145 | + | // loop (prepareSecurity): it ends or reschedules the retry. | |||
| 1139 | return true, nil | 1146 | return true, nil | ||
| 1140 | } | 1147 | } | ||
| 1141 | // Modeset on every enable: a fast disable+enable can coalesce to one true. | 1148 | // Modeset on every enable: a fast disable+enable can coalesce to one true. | ||
| @@ -1151,7 +1158,7 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||||
| 1151 | if !o.commitFailed(err, &enabled) { | 1158 | if !o.commitFailed(err, &enabled) { | ||
| 1152 | return true, err | 1159 | return true, err | ||
| 1153 | } | 1160 | } | ||
| 1154 | - | reacquire.refused(err, &enabled) | 1161 | + | refuse(err) |
| 1155 | return true, nil // epoch retry or wait disabled for master reacquisition | 1162 | return true, nil // epoch retry or wait disabled for master reacquisition | ||
| 1156 | } | 1163 | } | ||
| 1157 | reacquire.done() | 1164 | reacquire.done() | ||
| @@ -1186,6 +1193,11 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||||
| 1186 | if !o.commitFailed(err, &enabled) { | 1193 | if !o.commitFailed(err, &enabled) { | ||
| 1187 | return err | 1194 | return err | ||
| 1188 | } | 1195 | } | ||
| 1196 | + | // A lock screen refused for lack of DRM master after a VT | |||
| 1197 | + | // switch is retried like an ordinary enable. | |||
| 1198 | + | refuse(err) | |||
| 1199 | + | } else { | |||
| 1200 | + | reacquire.done() | |||
| 1189 | } | 1201 | } | ||
| 1190 | // Recheck immediately: a takeover during the clear needs its own | 1202 | // Recheck immediately: a takeover during the clear needs its own | ||
| 1191 | // transaction, not another scene or wake to make progress. | 1203 | // transaction, not another scene or wake to make progress. | ||
| @@ -1273,7 +1285,7 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||||
| 1273 | } | 1285 | } | ||
| 1274 | // An enable refused for lack of DRM master is retried on time. | 1286 | // An enable refused for lack of DRM master is retried on time. | ||
| 1275 | var reacquireDue <-chan time.Time | 1287 | var reacquireDue <-chan time.Time | ||
| 1276 | - | if d, ok := reacquire.next(); ok { | 1288 | + | if d, ok := reacquire.next(time.Now()); ok { |
| 1277 | reacquireTimer.Reset(d) | 1289 | reacquireTimer.Reset(d) | ||
| 1278 | reacquireDue = reacquireTimer.C | 1290 | reacquireDue = reacquireTimer.C | ||
| 1279 | } else { | 1291 | } else { | ||
| @@ -1342,7 +1354,9 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||||
| 1342 | case ack := <-o.seatDisable: | 1354 | case ack := <-o.seatDisable: | ||
| 1343 | o.bypassForSeatDisable() | 1355 | o.bypassForSeatDisable() | ||
| 1344 | // The seat is about to be disabled: no frame may select the | 1356 | // The seat is about to be disabled: no frame may select the | ||
| 1345 | - | // pipeline again before the next enable (a modeset). | 1357 | + | // pipeline again before the next enable (a modeset), not even a |
| 1358 | + | // retried one. | |||
| 1359 | + | reacquire.done() | |||
| 1346 | enabled = false | 1360 | enabled = false | ||
| 1347 | ack <- struct{}{} | 1361 | ack <- struct{}{} | ||
| 1348 | continue | 1362 | continue | ||
| @@ -1359,7 +1373,7 @@ func (o *Output) Run(ctx context.Context, newRenderer func(w, h int) (ports.Rend | |||||
| 1359 | case <-reacquireDue: | 1373 | case <-reacquireDue: | ||
| 1360 | // The seat reported active before the kernel gave DRM master | 1374 | // The seat reported active before the kernel gave DRM master | ||
| 1361 | // back: the enable modeset was refused. Try it again. | 1375 | // back: the enable modeset was refused. Try it again. | ||
| 1362 | - | reacquire.attempts++ | 1376 | + | reacquire.fire() |
| 1363 | skip, err := setActive(true) | 1377 | skip, err := setActive(true) | ||
| 1364 | if err != nil { | 1378 | if err != nil { | ||
| 1365 | return err | 1379 | return err | ||
M internal/adapters/drm/reacquire.go +28 -11
| @@ -6,34 +6,51 @@ import "time" | |||
| 6 | 6 | // DRM master was not back yet. The seat can report the session active | |
| 7 | 7 | // before the kernel hands master back; without a retry the output stays | |
| 8 | 8 | // dark until the next VT switch. | |
| 9 | + | // | |
| 10 | + | // The retry has a fixed deadline: the output loop wakes for many other | |
| 11 | + | // events (pings, cursor, reports) and must not push it back on each one. | |
| 9 | 12 | type masterRetry struct { | |
| 10 | 13 | pending bool | |
| 14 | + | due time.Time | |
| 11 | 15 | attempts int | |
| 12 | 16 | } | |
| 13 | 17 | ||
| 14 | 18 | const ( | |
| 15 | 19 | masterRetryFirst = 20 * time.Millisecond | |
| 16 | 20 | masterRetryMax = time.Second | |
| 17 | - | // masterRetryLimit bounds the retries of one enable (about 30 s). | |
| 21 | + | // masterRetryLimit bounds the retries of one enable (about 35 s). | |
| 18 | 22 | masterRetryLimit = 40 | |
| 19 | 23 | ) | |
| 20 | 24 | ||
| 21 | - | // refused records an enable refused with err; only a lost master is | |
| 22 | - | // retried, and enabled stays false until a retry succeeds. | |
| 23 | - | func (m *masterRetry) refused(err error, enabled *bool) { | |
| 24 | - | if !lostMaster(err) || *enabled { | |
| 25 | - | return | |
| 25 | + | // refused records an enable refused with err at now; only a lost master | |
| 26 | + | // is retried, and enabled stays false until a retry succeeds. It reports | |
| 27 | + | // that the retries are exhausted. | |
| 28 | + | func (m *masterRetry) refused(err error, enabled bool, now time.Time) (gaveUp bool) { | |
| 29 | + | if !lostMaster(err) || enabled { | |
| 30 | + | return false | |
| 31 | + | } | |
| 32 | + | if m.attempts >= masterRetryLimit { | |
| 33 | + | m.pending = false | |
| 34 | + | return true | |
| 26 | 35 | } | |
| 27 | 36 | m.pending = true | |
| 37 | + | m.due = now.Add(min(masterRetryFirst<<min(m.attempts, 6), masterRetryMax)) | |
| 38 | + | return false | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | // fire starts an attempt: the retry is pending again only if it is refused. | |
| 42 | + | func (m *masterRetry) fire() { | |
| 43 | + | m.pending = false | |
| 44 | + | m.attempts++ | |
| 28 | 45 | } | |
| 29 | 46 | ||
| 30 | - | // done ends the retry: the enable succeeded or the seat changed again. | |
| 47 | + | // done ends the retry: the enable succeeded or the seat changed. | |
| 31 | 48 | func (m *masterRetry) done() { *m = masterRetry{} } | |
| 32 | 49 | ||
| 33 | - | // next is the delay before the next attempt, if one is due. | |
| 34 | - | func (m *masterRetry) next() (time.Duration, bool) { | |
| 35 | - | if !m.pending || m.attempts >= masterRetryLimit { | |
| 50 | + | // next is the time left before the next attempt, if one is due. | |
| 51 | + | func (m *masterRetry) next(now time.Time) (time.Duration, bool) { | |
| 52 | + | if !m.pending { | |
| 36 | 53 | return 0, false | |
| 37 | 54 | } | |
| 38 | - | return min(masterRetryFirst<<min(m.attempts, 6), masterRetryMax), true | |
| 55 | + | return max(0, m.due.Sub(now)), true | |
| 39 | 56 | } | |
| @@ -6,34 +6,51 @@ import "time" | |||||
| 6 | // DRM master was not back yet. The seat can report the session active | 6 | // DRM master was not back yet. The seat can report the session active | ||
| 7 | // before the kernel hands master back; without a retry the output stays | 7 | // before the kernel hands master back; without a retry the output stays | ||
| 8 | // dark until the next VT switch. | 8 | // dark until the next VT switch. | ||
| 9 | + | // | |||
| 10 | + | // The retry has a fixed deadline: the output loop wakes for many other | |||
| 11 | + | // events (pings, cursor, reports) and must not push it back on each one. | |||
| 9 | type masterRetry struct { | 12 | type masterRetry struct { | ||
| 10 | pending bool | 13 | pending bool | ||
| 14 | + | due time.Time | |||
| 11 | attempts int | 15 | attempts int | ||
| 12 | } | 16 | } | ||
| 13 | 17 | ||||
| 14 | const ( | 18 | const ( | ||
| 15 | masterRetryFirst = 20 * time.Millisecond | 19 | masterRetryFirst = 20 * time.Millisecond | ||
| 16 | masterRetryMax = time.Second | 20 | masterRetryMax = time.Second | ||
| 17 | - | // masterRetryLimit bounds the retries of one enable (about 30 s). | 21 | + | // masterRetryLimit bounds the retries of one enable (about 35 s). |
| 18 | masterRetryLimit = 40 | 22 | masterRetryLimit = 40 | ||
| 19 | ) | 23 | ) | ||
| 20 | 24 | ||||
| 21 | - | // refused records an enable refused with err; only a lost master is | 25 | + | // refused records an enable refused with err at now; only a lost master |
| 22 | - | // retried, and enabled stays false until a retry succeeds. | 26 | + | // is retried, and enabled stays false until a retry succeeds. It reports |
| 23 | - | func (m *masterRetry) refused(err error, enabled *bool) { | 27 | + | // that the retries are exhausted. |
| 24 | - | if !lostMaster(err) || *enabled { | 28 | + | func (m *masterRetry) refused(err error, enabled bool, now time.Time) (gaveUp bool) { |
| 25 | - | return | 29 | + | if !lostMaster(err) || enabled { |
| 30 | + | return false | |||
| 31 | + | } | |||
| 32 | + | if m.attempts >= masterRetryLimit { | |||
| 33 | + | m.pending = false | |||
| 34 | + | return true | |||
| 26 | } | 35 | } | ||
| 27 | m.pending = true | 36 | m.pending = true | ||
| 37 | + | m.due = now.Add(min(masterRetryFirst<<min(m.attempts, 6), masterRetryMax)) | |||
| 38 | + | return false | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | // fire starts an attempt: the retry is pending again only if it is refused. | |||
| 42 | + | func (m *masterRetry) fire() { | |||
| 43 | + | m.pending = false | |||
| 44 | + | m.attempts++ | |||
| 28 | } | 45 | } | ||
| 29 | 46 | ||||
| 30 | - | // done ends the retry: the enable succeeded or the seat changed again. | 47 | + | // done ends the retry: the enable succeeded or the seat changed. |
| 31 | func (m *masterRetry) done() { *m = masterRetry{} } | 48 | func (m *masterRetry) done() { *m = masterRetry{} } | ||
| 32 | 49 | ||||
| 33 | - | // next is the delay before the next attempt, if one is due. | 50 | + | // next is the time left before the next attempt, if one is due. |
| 34 | - | func (m *masterRetry) next() (time.Duration, bool) { | 51 | + | func (m *masterRetry) next(now time.Time) (time.Duration, bool) { |
| 35 | - | if !m.pending || m.attempts >= masterRetryLimit { | 52 | + | if !m.pending { |
| 36 | return 0, false | 53 | return 0, false | ||
| 37 | } | 54 | } | ||
| 38 | - | return min(masterRetryFirst<<min(m.attempts, 6), masterRetryMax), true | 55 | + | return max(0, m.due.Sub(now)), true |
| 39 | } | 56 | } | ||
M internal/adapters/drm/reacquire_test.go +122 -54
| @@ -3,6 +3,7 @@ package drm | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | + | "sync" | |
| 6 | 7 | "testing" | |
| 7 | 8 | "testing/synctest" | |
| 8 | 9 | "time" | |
| @@ -15,43 +16,63 @@ import ( | |||
| 15 | 16 | ||
| 16 | 17 | func TestMasterRetryBackoff(t *testing.T) { | |
| 17 | 18 | var m masterRetry | |
| 18 | - | enabled := false | |
| 19 | - | if _, ok := m.next(); ok { | |
| 19 | + | now := time.Unix(0, 0) | |
| 20 | + | if _, ok := m.next(now); ok { | |
| 20 | 21 | t.Fatal("retry due before a refusal") | |
| 21 | 22 | } | |
| 22 | - | m.refused(unix.EBUSY, &enabled) | |
| 23 | - | if _, ok := m.next(); ok { | |
| 23 | + | if m.refused(unix.EBUSY, false, now); m.pending { | |
| 24 | 24 | t.Fatal("a busy refusal is retried by the busy path, not here") | |
| 25 | 25 | } | |
| 26 | - | m.refused(unix.EACCES, &enabled) | |
| 27 | - | d, ok := m.next() | |
| 28 | - | if !ok || d != masterRetryFirst { | |
| 26 | + | if m.refused(unix.EACCES, true, now); m.pending { | |
| 27 | + | t.Fatal("an enabled output is not retried") | |
| 28 | + | } | |
| 29 | + | m.refused(unix.EACCES, false, now) | |
| 30 | + | // The deadline is fixed: asking later leaves less, never more. | |
| 31 | + | if d, ok := m.next(now); !ok || d != masterRetryFirst { | |
| 29 | 32 | t.Fatalf("first retry %v %v", d, ok) | |
| 30 | 33 | } | |
| 34 | + | if d, _ := m.next(now.Add(masterRetryFirst / 2)); d != masterRetryFirst/2 { | |
| 35 | + | t.Fatalf("half-way %v", d) | |
| 36 | + | } | |
| 37 | + | if d, _ := m.next(now.Add(time.Hour)); d != 0 { | |
| 38 | + | t.Fatalf("overdue %v", d) | |
| 39 | + | } | |
| 40 | + | m.fire() | |
| 41 | + | if _, ok := m.next(now); ok { | |
| 42 | + | t.Fatal("pending while an attempt runs") | |
| 43 | + | } | |
| 31 | 44 | m.attempts = 30 | |
| 32 | - | if d, _ := m.next(); d != masterRetryMax { | |
| 45 | + | m.refused(unix.EPERM, false, now) | |
| 46 | + | if d, _ := m.next(now); d != masterRetryMax { | |
| 33 | 47 | t.Fatalf("capped retry %v", d) | |
| 34 | 48 | } | |
| 35 | 49 | m.attempts = masterRetryLimit | |
| 36 | - | if _, ok := m.next(); ok { | |
| 50 | + | if !m.refused(unix.EACCES, false, now) || m.pending { | |
| 37 | 51 | t.Fatal("retries past the limit") | |
| 38 | 52 | } | |
| 39 | 53 | m.done() | |
| 40 | - | if _, ok := m.next(); ok { | |
| 54 | + | if _, ok := m.next(now); ok || m.attempts != 0 { | |
| 41 | 55 | t.Fatal("retry after done") | |
| 42 | 56 | } | |
| 43 | 57 | } | |
| 44 | 58 | ||
| 45 | - | // The seat reports active before the kernel gives DRM master back: the | |
| 46 | - | // enable modeset is refused, then retried until it succeeds, without a | |
| 47 | - | // second seat change. | |
| 48 | - | func TestEnableRefusedForMasterIsRetried(t *testing.T) { | |
| 49 | - | synctest.Test(t, enableRefusedForMasterIsRetried) | |
| 59 | + | // retryHarness runs an output whose enable modesets the kernel refuses | |
| 60 | + | // (lost master) until allow is called. | |
| 61 | + | type retryHarness struct { | |
| 62 | + | o *Output | |
| 63 | + | active chan bool | |
| 64 | + | modesets func() int | |
| 65 | + | allow func() | |
| 66 | + | stop func() | |
| 50 | 67 | } | |
| 51 | 68 | ||
| 52 | - | func enableRefusedForMasterIsRetried(t *testing.T) { | |
| 69 | + | func newRetryHarness(t *testing.T) retryHarness { | |
| 70 | + | t.Helper() | |
| 71 | + | var mu sync.Mutex | |
| 53 | 72 | refuse := true | |
| 54 | 73 | o, k, commits, commitMu := testOutputRules(t, func(_ *atomicReq, flags uint32) error { | |
| 74 | + | mu.Lock() | |
| 75 | + | defer mu.Unlock() | |
| 55 | 76 | if refuse && flags&atomicAllowModes != 0 && flags&atomicTestOnly == 0 { | |
| 56 | 77 | return unix.EACCES | |
| 57 | 78 | } | |
| @@ -63,9 +84,13 @@ func enableRefusedForMasterIsRetried(t *testing.T) { | |||
| 63 | 84 | r.EXPECT().UseTarget(mock.Anything).Return().Maybe() | |
| 64 | 85 | r.EXPECT().Render(mock.Anything, mock.Anything).Return(nil, nil).Maybe() | |
| 65 | 86 | r.EXPECT().Close().Return().Once() | |
| 87 | + | // The 10 s stats tick runs in long tests. | |
| 88 | + | r.EXPECT().Trim(mock.Anything).Return(nil).Maybe() | |
| 89 | + | r.EXPECT().TakeRedrawn().Return(0).Maybe() | |
| 66 | 90 | k.EXPECT().createBlob(mock.Anything).Return(99, nil).Maybe() | |
| 67 | 91 | k.EXPECT().destroyBlob(mock.Anything).Return(nil).Maybe() | |
| 68 | 92 | k.EXPECT().rmFB(mock.Anything).Return(nil).Maybe() | |
| 93 | + | o.ping = make(chan chan struct{}) | |
| 69 | 94 | active := make(chan bool, 1) | |
| 70 | 95 | active <- false | |
| 71 | 96 | ctx, cancel := context.WithCancel(context.Background()) | |
| @@ -73,44 +98,87 @@ func enableRefusedForMasterIsRetried(t *testing.T) { | |||
| 73 | 98 | go func() { | |
| 74 | 99 | done <- o.Run(ctx, func(int, int) (ports.Renderer, error) { return r, nil }, nil, active, nil, nil, nil, make(chan ports.OutputPresented, 1), nil, nil) | |
| 75 | 100 | }() | |
| 76 | - | modesets := func() int { | |
| 77 | - | commitMu.Lock() | |
| 78 | - | defer commitMu.Unlock() | |
| 79 | - | n := 0 | |
| 80 | - | for _, c := range *commits { | |
| 81 | - | if c.flags&atomicAllowModes != 0 && c.flags&atomicTestOnly == 0 { | |
| 82 | - | n++ | |
| 83 | - | } | |
| 84 | - | } | |
| 85 | - | return n | |
| 86 | - | } | |
| 87 | - | synctest.Wait() | |
| 88 | - | active <- true | |
| 89 | 101 | synctest.Wait() | |
| 90 | - | if n := modesets(); n != 1 { | |
| 91 | - | t.Fatalf("%d modesets after the refused enable", n) | |
| 92 | - | } | |
| 93 | - | time.Sleep(masterRetryFirst) | |
| 94 | - | synctest.Wait() | |
| 95 | - | if n := modesets(); n != 2 { | |
| 96 | - | t.Fatalf("%d modesets after the first retry", n) | |
| 97 | - | } | |
| 98 | - | commitMu.Lock() | |
| 99 | - | refuse = false | |
| 100 | - | commitMu.Unlock() | |
| 101 | - | time.Sleep(2 * masterRetryFirst) | |
| 102 | - | synctest.Wait() | |
| 103 | - | if n := modesets(); n != 3 { | |
| 104 | - | t.Fatalf("%d modesets after the retry that succeeds", n) | |
| 105 | - | } | |
| 106 | - | // Once enabled, nothing retries any more. | |
| 107 | - | time.Sleep(masterRetryMax * 2) | |
| 108 | - | synctest.Wait() | |
| 109 | - | if n := modesets(); n != 3 { | |
| 110 | - | t.Fatalf("%d modesets after success", n) | |
| 111 | - | } | |
| 112 | - | cancel() | |
| 113 | - | if err := <-done; err != nil && !errors.Is(err, context.Canceled) { | |
| 114 | - | t.Fatal(err) | |
| 102 | + | return retryHarness{ | |
| 103 | + | o: o, active: active, | |
| 104 | + | modesets: func() int { | |
| 105 | + | commitMu.Lock() | |
| 106 | + | defer commitMu.Unlock() | |
| 107 | + | n := 0 | |
| 108 | + | for _, c := range *commits { | |
| 109 | + | if c.flags&atomicAllowModes != 0 && c.flags&atomicTestOnly == 0 { | |
| 110 | + | n++ | |
| 111 | + | } | |
| 112 | + | } | |
| 113 | + | return n | |
| 114 | + | }, | |
| 115 | + | allow: func() { mu.Lock(); refuse = false; mu.Unlock() }, | |
| 116 | + | stop: func() { | |
| 117 | + | cancel() | |
| 118 | + | if err := <-done; err != nil && !errors.Is(err, context.Canceled) { | |
| 119 | + | t.Fatal(err) | |
| 120 | + | } | |
| 121 | + | }, | |
| 115 | 122 | } | |
| 116 | 123 | } | |
| 124 | + | ||
| 125 | + | // The seat reports active before the kernel gives DRM master back: the | |
| 126 | + | // enable modeset is refused, then retried until it succeeds, without a | |
| 127 | + | // second seat change, even while other events (watchdog pings) keep | |
| 128 | + | // waking the loop faster than the backoff. | |
| 129 | + | func TestEnableRefusedForMasterIsRetried(t *testing.T) { | |
| 130 | + | synctest.Test(t, func(t *testing.T) { | |
| 131 | + | h := newRetryHarness(t) | |
| 132 | + | h.active <- true | |
| 133 | + | synctest.Wait() | |
| 134 | + | if n := h.modesets(); n != 1 { | |
| 135 | + | t.Fatalf("%d modesets after the refused enable", n) | |
| 136 | + | } | |
| 137 | + | // Let the backoff grow past the ping period. | |
| 138 | + | pingFor := func(d time.Duration) { | |
| 139 | + | for end := time.Now().Add(d); time.Now().Before(end); { | |
| 140 | + | time.Sleep(100 * time.Millisecond) | |
| 141 | + | h.o.Ping(context.Background()) | |
| 142 | + | } | |
| 143 | + | synctest.Wait() | |
| 144 | + | } | |
| 145 | + | pingFor(3 * time.Second) | |
| 146 | + | before := h.modesets() | |
| 147 | + | if before < 6 { | |
| 148 | + | t.Fatalf("%d modesets in 3 s of pings: the retry was pushed back", before) | |
| 149 | + | } | |
| 150 | + | h.allow() | |
| 151 | + | pingFor(2 * masterRetryMax) | |
| 152 | + | after := h.modesets() | |
| 153 | + | if after != before+1 { | |
| 154 | + | t.Fatalf("%d modesets before success, %d after", before, after) | |
| 155 | + | } | |
| 156 | + | // Once enabled, nothing retries any more. | |
| 157 | + | pingFor(2 * masterRetryMax) | |
| 158 | + | if n := h.modesets(); n != after { | |
| 159 | + | t.Fatalf("%d modesets after success", n) | |
| 160 | + | } | |
| 161 | + | h.stop() | |
| 162 | + | }) | |
| 163 | + | } | |
| 164 | + | ||
| 165 | + | // A seat disable ends a pending retry: no modeset may follow the plane | |
| 166 | + | // reset before the next enable. | |
| 167 | + | func TestSeatDisableCancelsMasterRetry(t *testing.T) { | |
| 168 | + | synctest.Test(t, func(t *testing.T) { | |
| 169 | + | h := newRetryHarness(t) | |
| 170 | + | h.active <- true | |
| 171 | + | synctest.Wait() | |
| 172 | + | ack := make(chan struct{}) | |
| 173 | + | h.o.seatDisable <- ack | |
| 174 | + | <-ack | |
| 175 | + | synctest.Wait() | |
| 176 | + | n := h.modesets() | |
| 177 | + | time.Sleep(10 * masterRetryMax) | |
| 178 | + | synctest.Wait() | |
| 179 | + | if got := h.modesets(); got != n { | |
| 180 | + | t.Fatalf("%d modesets after the seat disable, want %d", got, n) | |
| 181 | + | } | |
| 182 | + | h.stop() | |
| 183 | + | }) | |
| 184 | + | } | |
| @@ -3,6 +3,7 @@ package drm | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | + | "sync" | |||
| 6 | "testing" | 7 | "testing" | ||
| 7 | "testing/synctest" | 8 | "testing/synctest" | ||
| 8 | "time" | 9 | "time" | ||
| @@ -15,43 +16,63 @@ import ( | |||||
| 15 | 16 | ||||
| 16 | func TestMasterRetryBackoff(t *testing.T) { | 17 | func TestMasterRetryBackoff(t *testing.T) { | ||
| 17 | var m masterRetry | 18 | var m masterRetry | ||
| 18 | - | enabled := false | 19 | + | now := time.Unix(0, 0) |
| 19 | - | if _, ok := m.next(); ok { | 20 | + | if _, ok := m.next(now); ok { |
| 20 | t.Fatal("retry due before a refusal") | 21 | t.Fatal("retry due before a refusal") | ||
| 21 | } | 22 | } | ||
| 22 | - | m.refused(unix.EBUSY, &enabled) | 23 | + | if m.refused(unix.EBUSY, false, now); m.pending { |
| 23 | - | if _, ok := m.next(); ok { | |||
| 24 | t.Fatal("a busy refusal is retried by the busy path, not here") | 24 | t.Fatal("a busy refusal is retried by the busy path, not here") | ||
| 25 | } | 25 | } | ||
| 26 | - | m.refused(unix.EACCES, &enabled) | 26 | + | if m.refused(unix.EACCES, true, now); m.pending { |
| 27 | - | d, ok := m.next() | 27 | + | t.Fatal("an enabled output is not retried") |
| 28 | - | if !ok || d != masterRetryFirst { | 28 | + | } |
| 29 | + | m.refused(unix.EACCES, false, now) | |||
| 30 | + | // The deadline is fixed: asking later leaves less, never more. | |||
| 31 | + | if d, ok := m.next(now); !ok || d != masterRetryFirst { | |||
| 29 | t.Fatalf("first retry %v %v", d, ok) | 32 | t.Fatalf("first retry %v %v", d, ok) | ||
| 30 | } | 33 | } | ||
| 34 | + | if d, _ := m.next(now.Add(masterRetryFirst / 2)); d != masterRetryFirst/2 { | |||
| 35 | + | t.Fatalf("half-way %v", d) | |||
| 36 | + | } | |||
| 37 | + | if d, _ := m.next(now.Add(time.Hour)); d != 0 { | |||
| 38 | + | t.Fatalf("overdue %v", d) | |||
| 39 | + | } | |||
| 40 | + | m.fire() | |||
| 41 | + | if _, ok := m.next(now); ok { | |||
| 42 | + | t.Fatal("pending while an attempt runs") | |||
| 43 | + | } | |||
| 31 | m.attempts = 30 | 44 | m.attempts = 30 | ||
| 32 | - | if d, _ := m.next(); d != masterRetryMax { | 45 | + | m.refused(unix.EPERM, false, now) |
| 46 | + | if d, _ := m.next(now); d != masterRetryMax { | |||
| 33 | t.Fatalf("capped retry %v", d) | 47 | t.Fatalf("capped retry %v", d) | ||
| 34 | } | 48 | } | ||
| 35 | m.attempts = masterRetryLimit | 49 | m.attempts = masterRetryLimit | ||
| 36 | - | if _, ok := m.next(); ok { | 50 | + | if !m.refused(unix.EACCES, false, now) || m.pending { |
| 37 | t.Fatal("retries past the limit") | 51 | t.Fatal("retries past the limit") | ||
| 38 | } | 52 | } | ||
| 39 | m.done() | 53 | m.done() | ||
| 40 | - | if _, ok := m.next(); ok { | 54 | + | if _, ok := m.next(now); ok || m.attempts != 0 { |
| 41 | t.Fatal("retry after done") | 55 | t.Fatal("retry after done") | ||
| 42 | } | 56 | } | ||
| 43 | } | 57 | } | ||
| 44 | 58 | ||||
| 45 | - | // The seat reports active before the kernel gives DRM master back: the | 59 | + | // retryHarness runs an output whose enable modesets the kernel refuses |
| 46 | - | // enable modeset is refused, then retried until it succeeds, without a | 60 | + | // (lost master) until allow is called. |
| 47 | - | // second seat change. | 61 | + | type retryHarness struct { |
| 48 | - | func TestEnableRefusedForMasterIsRetried(t *testing.T) { | 62 | + | o *Output |
| 49 | - | synctest.Test(t, enableRefusedForMasterIsRetried) | 63 | + | active chan bool |
| 64 | + | modesets func() int | |||
| 65 | + | allow func() | |||
| 66 | + | stop func() | |||
| 50 | } | 67 | } | ||
| 51 | 68 | ||||
| 52 | - | func enableRefusedForMasterIsRetried(t *testing.T) { | 69 | + | func newRetryHarness(t *testing.T) retryHarness { |
| 70 | + | t.Helper() | |||
| 71 | + | var mu sync.Mutex | |||
| 53 | refuse := true | 72 | refuse := true | ||
| 54 | o, k, commits, commitMu := testOutputRules(t, func(_ *atomicReq, flags uint32) error { | 73 | o, k, commits, commitMu := testOutputRules(t, func(_ *atomicReq, flags uint32) error { | ||
| 74 | + | mu.Lock() | |||
| 75 | + | defer mu.Unlock() | |||
| 55 | if refuse && flags&atomicAllowModes != 0 && flags&atomicTestOnly == 0 { | 76 | if refuse && flags&atomicAllowModes != 0 && flags&atomicTestOnly == 0 { | ||
| 56 | return unix.EACCES | 77 | return unix.EACCES | ||
| 57 | } | 78 | } | ||
| @@ -63,9 +84,13 @@ func enableRefusedForMasterIsRetried(t *testing.T) { | |||||
| 63 | r.EXPECT().UseTarget(mock.Anything).Return().Maybe() | 84 | r.EXPECT().UseTarget(mock.Anything).Return().Maybe() | ||
| 64 | r.EXPECT().Render(mock.Anything, mock.Anything).Return(nil, nil).Maybe() | 85 | r.EXPECT().Render(mock.Anything, mock.Anything).Return(nil, nil).Maybe() | ||
| 65 | r.EXPECT().Close().Return().Once() | 86 | r.EXPECT().Close().Return().Once() | ||
| 87 | + | // The 10 s stats tick runs in long tests. | |||
| 88 | + | r.EXPECT().Trim(mock.Anything).Return(nil).Maybe() | |||
| 89 | + | r.EXPECT().TakeRedrawn().Return(0).Maybe() | |||
| 66 | k.EXPECT().createBlob(mock.Anything).Return(99, nil).Maybe() | 90 | k.EXPECT().createBlob(mock.Anything).Return(99, nil).Maybe() | ||
| 67 | k.EXPECT().destroyBlob(mock.Anything).Return(nil).Maybe() | 91 | k.EXPECT().destroyBlob(mock.Anything).Return(nil).Maybe() | ||
| 68 | k.EXPECT().rmFB(mock.Anything).Return(nil).Maybe() | 92 | k.EXPECT().rmFB(mock.Anything).Return(nil).Maybe() | ||
| 93 | + | o.ping = make(chan chan struct{}) | |||
| 69 | active := make(chan bool, 1) | 94 | active := make(chan bool, 1) | ||
| 70 | active <- false | 95 | active <- false | ||
| 71 | ctx, cancel := context.WithCancel(context.Background()) | 96 | ctx, cancel := context.WithCancel(context.Background()) | ||
| @@ -73,44 +98,87 @@ func enableRefusedForMasterIsRetried(t *testing.T) { | |||||
| 73 | go func() { | 98 | go func() { | ||
| 74 | done <- o.Run(ctx, func(int, int) (ports.Renderer, error) { return r, nil }, nil, active, nil, nil, nil, make(chan ports.OutputPresented, 1), nil, nil) | 99 | done <- o.Run(ctx, func(int, int) (ports.Renderer, error) { return r, nil }, nil, active, nil, nil, nil, make(chan ports.OutputPresented, 1), nil, nil) | ||
| 75 | }() | 100 | }() | ||
| 76 | - | modesets := func() int { | |||
| 77 | - | commitMu.Lock() | |||
| 78 | - | defer commitMu.Unlock() | |||
| 79 | - | n := 0 | |||
| 80 | - | for _, c := range *commits { | |||
| 81 | - | if c.flags&atomicAllowModes != 0 && c.flags&atomicTestOnly == 0 { | |||
| 82 | - | n++ | |||
| 83 | - | } | |||
| 84 | - | } | |||
| 85 | - | return n | |||
| 86 | - | } | |||
| 87 | - | synctest.Wait() | |||
| 88 | - | active <- true | |||
| 89 | synctest.Wait() | 101 | synctest.Wait() | ||
| 90 | - | if n := modesets(); n != 1 { | 102 | + | return retryHarness{ |
| 91 | - | t.Fatalf("%d modesets after the refused enable", n) | 103 | + | o: o, active: active, |
| 92 | - | } | 104 | + | modesets: func() int { |
| 93 | - | time.Sleep(masterRetryFirst) | 105 | + | commitMu.Lock() |
| 94 | - | synctest.Wait() | 106 | + | defer commitMu.Unlock() |
| 95 | - | if n := modesets(); n != 2 { | 107 | + | n := 0 |
| 96 | - | t.Fatalf("%d modesets after the first retry", n) | 108 | + | for _, c := range *commits { |
| 97 | - | } | 109 | + | if c.flags&atomicAllowModes != 0 && c.flags&atomicTestOnly == 0 { |
| 98 | - | commitMu.Lock() | 110 | + | n++ |
| 99 | - | refuse = false | 111 | + | } |
| 100 | - | commitMu.Unlock() | 112 | + | } |
| 101 | - | time.Sleep(2 * masterRetryFirst) | 113 | + | return n |
| 102 | - | synctest.Wait() | 114 | + | }, |
| 103 | - | if n := modesets(); n != 3 { | 115 | + | allow: func() { mu.Lock(); refuse = false; mu.Unlock() }, |
| 104 | - | t.Fatalf("%d modesets after the retry that succeeds", n) | 116 | + | stop: func() { |
| 105 | - | } | 117 | + | cancel() |
| 106 | - | // Once enabled, nothing retries any more. | 118 | + | if err := <-done; err != nil && !errors.Is(err, context.Canceled) { |
| 107 | - | time.Sleep(masterRetryMax * 2) | 119 | + | t.Fatal(err) |
| 108 | - | synctest.Wait() | 120 | + | } |
| 109 | - | if n := modesets(); n != 3 { | 121 | + | }, |
| 110 | - | t.Fatalf("%d modesets after success", n) | |||
| 111 | - | } | |||
| 112 | - | cancel() | |||
| 113 | - | if err := <-done; err != nil && !errors.Is(err, context.Canceled) { | |||
| 114 | - | t.Fatal(err) | |||
| 115 | } | 122 | } | ||
| 116 | } | 123 | } | ||
| 124 | + | ||||
| 125 | + | // The seat reports active before the kernel gives DRM master back: the | |||
| 126 | + | // enable modeset is refused, then retried until it succeeds, without a | |||
| 127 | + | // second seat change, even while other events (watchdog pings) keep | |||
| 128 | + | // waking the loop faster than the backoff. | |||
| 129 | + | func TestEnableRefusedForMasterIsRetried(t *testing.T) { | |||
| 130 | + | synctest.Test(t, func(t *testing.T) { | |||
| 131 | + | h := newRetryHarness(t) | |||
| 132 | + | h.active <- true | |||
| 133 | + | synctest.Wait() | |||
| 134 | + | if n := h.modesets(); n != 1 { | |||
| 135 | + | t.Fatalf("%d modesets after the refused enable", n) | |||
| 136 | + | } | |||
| 137 | + | // Let the backoff grow past the ping period. | |||
| 138 | + | pingFor := func(d time.Duration) { | |||
| 139 | + | for end := time.Now().Add(d); time.Now().Before(end); { | |||
| 140 | + | time.Sleep(100 * time.Millisecond) | |||
| 141 | + | h.o.Ping(context.Background()) | |||
| 142 | + | } | |||
| 143 | + | synctest.Wait() | |||
| 144 | + | } | |||
| 145 | + | pingFor(3 * time.Second) | |||
| 146 | + | before := h.modesets() | |||
| 147 | + | if before < 6 { | |||
| 148 | + | t.Fatalf("%d modesets in 3 s of pings: the retry was pushed back", before) | |||
| 149 | + | } | |||
| 150 | + | h.allow() | |||
| 151 | + | pingFor(2 * masterRetryMax) | |||
| 152 | + | after := h.modesets() | |||
| 153 | + | if after != before+1 { | |||
| 154 | + | t.Fatalf("%d modesets before success, %d after", before, after) | |||
| 155 | + | } | |||
| 156 | + | // Once enabled, nothing retries any more. | |||
| 157 | + | pingFor(2 * masterRetryMax) | |||
| 158 | + | if n := h.modesets(); n != after { | |||
| 159 | + | t.Fatalf("%d modesets after success", n) | |||
| 160 | + | } | |||
| 161 | + | h.stop() | |||
| 162 | + | }) | |||
| 163 | + | } | |||
| 164 | + | ||||
| 165 | + | // A seat disable ends a pending retry: no modeset may follow the plane | |||
| 166 | + | // reset before the next enable. | |||
| 167 | + | func TestSeatDisableCancelsMasterRetry(t *testing.T) { | |||
| 168 | + | synctest.Test(t, func(t *testing.T) { | |||
| 169 | + | h := newRetryHarness(t) | |||
| 170 | + | h.active <- true | |||
| 171 | + | synctest.Wait() | |||
| 172 | + | ack := make(chan struct{}) | |||
| 173 | + | h.o.seatDisable <- ack | |||
| 174 | + | <-ack | |||
| 175 | + | synctest.Wait() | |||
| 176 | + | n := h.modesets() | |||
| 177 | + | time.Sleep(10 * masterRetryMax) | |||
| 178 | + | synctest.Wait() | |||
| 179 | + | if got := h.modesets(); got != n { | |||
| 180 | + | t.Fatalf("%d modesets after the seat disable, want %d", got, n) | |||
| 181 | + | } | |||
| 182 | + | h.stop() | |||
| 183 | + | }) | |||
| 184 | + | } | |||