fix(drm): reap signalled render fences during commit retries
2 files · +76 -1
A internal/adapters/drm/nomem_reads_test.go +73 -0
| @@ -0,0 +1,73 @@ | |||
| 1 | + | package drm | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "errors" | |
| 6 | + | "os" | |
| 7 | + | "testing" | |
| 8 | + | "testing/synctest" | |
| 9 | + | "time" | |
| 10 | + | ||
| 11 | + | portsmocks "github.com/bnema/neferwl/internal/mocks/ports" | |
| 12 | + | "github.com/bnema/neferwl/internal/ports" | |
| 13 | + | "github.com/stretchr/testify/mock" | |
| 14 | + | "golang.org/x/sys/unix" | |
| 15 | + | ) | |
| 16 | + | ||
| 17 | + | // A static scene produces no reportDirty wake. Memory retries must reap | |
| 18 | + | // completed reads themselves, without dropping an older unsignalled read. | |
| 19 | + | func TestNoMemRetriesReapSignalledReads(t *testing.T) { | |
| 20 | + | synctest.Test(t, func(t *testing.T) { | |
| 21 | + | errs := make([]error, 40) | |
| 22 | + | for i := range errs { | |
| 23 | + | errs[i] = unix.ENOMEM | |
| 24 | + | } | |
| 25 | + | o, _, commits := testOutput(t, errs...) | |
| 26 | + | defer o.dropRead() | |
| 27 | + | held, signal := fencePipe(t) | |
| 28 | + | o.holdRead(held) | |
| 29 | + | pending := o.readFences[0] | |
| 30 | + | r := portsmocks.NewMockRenderer(t) | |
| 31 | + | r.EXPECT().UseTarget(0).Return() | |
| 32 | + | r.EXPECT().Render(mock.Anything, mock.Anything).RunAndReturn(func(ports.Scene, map[ports.WindowID]ports.SurfaceContent) (*os.File, error) { | |
| 33 | + | f, w := fencePipe(t) | |
| 34 | + | if _, err := w.Write([]byte{1}); err != nil { | |
| 35 | + | t.Fatal(err) | |
| 36 | + | } | |
| 37 | + | return f, nil | |
| 38 | + | }).Times(len(errs)) | |
| 39 | + | enabled := true | |
| 40 | + | var previous *os.File | |
| 41 | + | for range errs { | |
| 42 | + | _, err := o.submitFrame(context.Background(), r, ports.Scene{Seq: 1}, nil, nil, nil, nil) | |
| 43 | + | if !errors.Is(err, unix.ENOMEM) || !o.commitFailed(err, &enabled) { | |
| 44 | + | t.Fatalf("refusal: %v", err) | |
| 45 | + | } | |
| 46 | + | if len(o.readFences) != 2 || o.readFences[0] != pending { | |
| 47 | + | t.Fatalf("retained %d fences; unsignalled read preserved=%v", len(o.readFences), len(o.readFences) > 0 && o.readFences[0] == pending) | |
| 48 | + | } | |
| 49 | + | if previous != nil { | |
| 50 | + | if _, err := previous.Stat(); !errors.Is(err, os.ErrClosed) { | |
| 51 | + | t.Fatalf("completed descriptor not closed: %v", err) | |
| 52 | + | } | |
| 53 | + | } | |
| 54 | + | previous = o.readFences[1] | |
| 55 | + | time.Sleep(o.frame.nomemBackoff) | |
| 56 | + | if o.expire() { | |
| 57 | + | t.Fatal("memory retry requested modeset") | |
| 58 | + | } | |
| 59 | + | } | |
| 60 | + | if len(*commits) != len(errs) { | |
| 61 | + | t.Fatalf("commits %d", len(*commits)) | |
| 62 | + | } | |
| 63 | + | if o.readDone() || len(o.readFences) != 1 || o.readFences[0] != pending { | |
| 64 | + | t.Fatal("unsignalled read lost") | |
| 65 | + | } | |
| 66 | + | if _, err := signal.Write([]byte{1}); err != nil { | |
| 67 | + | t.Fatal(err) | |
| 68 | + | } | |
| 69 | + | if !o.readDone() || len(o.readFences) != 0 { | |
| 70 | + | t.Fatal("completed read not reaped") | |
| 71 | + | } | |
| 72 | + | }) | |
| 73 | + | } | |
| @@ -0,0 +1,73 @@ | |||||
| 1 | + | package drm | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "errors" | |||
| 6 | + | "os" | |||
| 7 | + | "testing" | |||
| 8 | + | "testing/synctest" | |||
| 9 | + | "time" | |||
| 10 | + | ||||
| 11 | + | portsmocks "github.com/bnema/neferwl/internal/mocks/ports" | |||
| 12 | + | "github.com/bnema/neferwl/internal/ports" | |||
| 13 | + | "github.com/stretchr/testify/mock" | |||
| 14 | + | "golang.org/x/sys/unix" | |||
| 15 | + | ) | |||
| 16 | + | ||||
| 17 | + | // A static scene produces no reportDirty wake. Memory retries must reap | |||
| 18 | + | // completed reads themselves, without dropping an older unsignalled read. | |||
| 19 | + | func TestNoMemRetriesReapSignalledReads(t *testing.T) { | |||
| 20 | + | synctest.Test(t, func(t *testing.T) { | |||
| 21 | + | errs := make([]error, 40) | |||
| 22 | + | for i := range errs { | |||
| 23 | + | errs[i] = unix.ENOMEM | |||
| 24 | + | } | |||
| 25 | + | o, _, commits := testOutput(t, errs...) | |||
| 26 | + | defer o.dropRead() | |||
| 27 | + | held, signal := fencePipe(t) | |||
| 28 | + | o.holdRead(held) | |||
| 29 | + | pending := o.readFences[0] | |||
| 30 | + | r := portsmocks.NewMockRenderer(t) | |||
| 31 | + | r.EXPECT().UseTarget(0).Return() | |||
| 32 | + | r.EXPECT().Render(mock.Anything, mock.Anything).RunAndReturn(func(ports.Scene, map[ports.WindowID]ports.SurfaceContent) (*os.File, error) { | |||
| 33 | + | f, w := fencePipe(t) | |||
| 34 | + | if _, err := w.Write([]byte{1}); err != nil { | |||
| 35 | + | t.Fatal(err) | |||
| 36 | + | } | |||
| 37 | + | return f, nil | |||
| 38 | + | }).Times(len(errs)) | |||
| 39 | + | enabled := true | |||
| 40 | + | var previous *os.File | |||
| 41 | + | for range errs { | |||
| 42 | + | _, err := o.submitFrame(context.Background(), r, ports.Scene{Seq: 1}, nil, nil, nil, nil) | |||
| 43 | + | if !errors.Is(err, unix.ENOMEM) || !o.commitFailed(err, &enabled) { | |||
| 44 | + | t.Fatalf("refusal: %v", err) | |||
| 45 | + | } | |||
| 46 | + | if len(o.readFences) != 2 || o.readFences[0] != pending { | |||
| 47 | + | t.Fatalf("retained %d fences; unsignalled read preserved=%v", len(o.readFences), len(o.readFences) > 0 && o.readFences[0] == pending) | |||
| 48 | + | } | |||
| 49 | + | if previous != nil { | |||
| 50 | + | if _, err := previous.Stat(); !errors.Is(err, os.ErrClosed) { | |||
| 51 | + | t.Fatalf("completed descriptor not closed: %v", err) | |||
| 52 | + | } | |||
| 53 | + | } | |||
| 54 | + | previous = o.readFences[1] | |||
| 55 | + | time.Sleep(o.frame.nomemBackoff) | |||
| 56 | + | if o.expire() { | |||
| 57 | + | t.Fatal("memory retry requested modeset") | |||
| 58 | + | } | |||
| 59 | + | } | |||
| 60 | + | if len(*commits) != len(errs) { | |||
| 61 | + | t.Fatalf("commits %d", len(*commits)) | |||
| 62 | + | } | |||
| 63 | + | if o.readDone() || len(o.readFences) != 1 || o.readFences[0] != pending { | |||
| 64 | + | t.Fatal("unsignalled read lost") | |||
| 65 | + | } | |||
| 66 | + | if _, err := signal.Write([]byte{1}); err != nil { | |||
| 67 | + | t.Fatal(err) | |||
| 68 | + | } | |||
| 69 | + | if !o.readDone() || len(o.readFences) != 0 { | |||
| 70 | + | t.Fatal("completed read not reaped") | |||
| 71 | + | } | |||
| 72 | + | }) | |||
| 73 | + | } | |||
M internal/adapters/drm/reads.go +3 -1
| @@ -41,8 +41,10 @@ func signalled(fs []*os.File) bool { | |||
| 41 | 41 | return true | |
| 42 | 42 | } | |
| 43 | 43 | ||
| 44 | - | // holdRead keeps the fences of a frame that was not committed. | |
| 44 | + | // holdRead keeps the fences of a frame that was not committed. Reap old | |
| 45 | + | // completed reads even when no content publication asks for a report. | |
| 45 | 46 | func (o *Output) holdRead(fs ...*os.File) { | |
| 47 | + | o.readDone() | |
| 46 | 48 | o.readFences = append(o.readFences, dupFences(fs...)...) | |
| 47 | 49 | } | |
| 48 | 50 | ||
| @@ -41,8 +41,10 @@ func signalled(fs []*os.File) bool { | |||||
| 41 | return true | 41 | return true | ||
| 42 | } | 42 | } | ||
| 43 | 43 | ||||
| 44 | - | // holdRead keeps the fences of a frame that was not committed. | 44 | + | // holdRead keeps the fences of a frame that was not committed. Reap old |
| 45 | + | // completed reads even when no content publication asks for a report. | |||
| 45 | func (o *Output) holdRead(fs ...*os.File) { | 46 | func (o *Output) holdRead(fs ...*os.File) { | ||
| 47 | + | o.readDone() | |||
| 46 | o.readFences = append(o.readFences, dupFences(fs...)...) | 48 | o.readFences = append(o.readFences, dupFences(fs...)...) | ||
| 47 | } | 49 | } | ||
| 48 | 50 | ||||