[erykwieliczko]

<ai>
Fix deferred GEM-close cleanup removing new mappings when the same dma-buf is reimported. Found while investigating M3 Pro GPU crashes in Plasma Settings.

Use binding sequences to preserve newer mappings while retaining the indexed cleanup fast path. Also initialize the empty BO-unmap result.

Tested the complete series on M3 Pro with stock KWin: clean build and boot, 50/50 GPU regression cases, 10 Settings launches and 10 Breeze Light/Dark cycles. No GPU/MMU faults or compositor restarts.
</ai>

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

This PR limits deferred GEM-close cleanup to mappings covered by the close’s saved binding sequence.

• Deferred GEM closes leave newer mappings of the same object in place.

No actionable issues were found. The review checked source paths; it did not run a build or hardware tests.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Map succeeds] --> B[Stamp mapping and related records]
  C[GEM close] --> D[Save current binding sequence]
  D --> E[Wait for accepted jobs to retire]
  E --> F{BO has newer bindings?}
  F -->|No| G[Unmap through per-object list]
  F -->|Yes| H[Remove only mappings at or before saved sequence]
  G --> I[Remove matching older records]
  H --> I
  G -->|Allocation fails| H

Loading

_{Reviews (1) · Last reviewed commit: "drm/asahi: keep new bindings after defer..." · Reviewed by Greptile}

[joshuaswarren]

GREEN at 13c3371

Read: the full diff (7 files) and, at this head, mmu.rs, mmu/bind.rs and mmu/lifetime.rs around every changed line, plus drm_gpuvm_bo_unmap() in drm_gpuvm.c. I did not build it and did not run it; nothing below comes from a boot.

Lifetime and ordering, traced:

• Sequence source. One per-VM counter. A map step takes N = counter + 1 (checked add, EOVERFLOW on wrap) inside op_map, which runs under the VM exec lock, so sequences follow insertion order. newest_bind of the VM/BO pair is raised with fetch_max after the page-table work succeeded and before the GPUVA is inserted, still under that lock. A step that fails after taking N only leaves a gap.
• Every GPUVA carries a sequence. VmGpuVa is only built in op_map (new N) and op_remap (both pieces keep the original value), and VmGpuVa::default() is not used anywhere in mmu.rs, so there is no VA with an unset 0 that a later <= sequence test would sweep up by accident. Invariant I checked: for every VA, its sequence is at most newest_bind of its own VM/BO pair, because a remap piece belongs to the same BO as its parent.
• Close vs. reimport. The close records current_bind_sequence() and merges repeated closes of one object with max. A bind taken after that point has a larger N, so the deferred drain keeps it; a bind whose N was already taken is concurrent with the close and may be removed, which is a valid ordering. The context-view removal at close time (untrack_context_object_before) and the one after the drain use the same bound, and the shared-binding table uses the same test (entry.bind_sequence > sequence is kept).
• The fast path is safe. The newest_bind > sequence check happens after exec_lock, the same lock under which bind.rs runs sm_map and then commit_context_binding and track_shared_binding (both before drop(inner)), so no bind can slip between the check and bo_unmap. newest_bind never decreases, so a stale high value can only send a drain to the slower filtered walk, never to a wrong unmap. The exec lock and the BO are released before the fallback, as in drop_mappings_now.
• Between that release and the fallback's lock_inner() a bind can land; its VAs have N greater than the bound and are skipped by the predicate, so the gap is harmless. The ENOMEM fallbacks (exec_lock and bo_unmap) both go to the filtered walk, so the low-memory path no longer removes newer mappings either. The only remaining unconditional callers (drop_mappings_now and drop_mappings_low_memory) are the no-deferral path.
• Rust helper: unmap_object_where only adds a matches closure to the existing allocation-free walk; unmap_all and unmap_object pass |_| true, so their behaviour is unchanged.

drm_gpuvm.c: int ret = 0; is correct and needed. When the BO has no GPUVAs the ops list is empty, the loop never runs and the function returned an uninitialised int. Kernel style is fine for upstream (one line, no behaviour change otherwise). Caveat for upstreaming: I could not find drm_gpuvm_bo_unmap() (with its immediate-mode guard(mutex) branch) in mainline drm_gpuvm.c, so I read it as a function that lives in the Asahi tree; if so, the fix belongs with the series that adds it (a fixup or a Fixes: tag naming that commit) rather than as a standalone mainline patch. Unrelated and optional: its kernel-doc says "Returns: a pointer to the &drm_gpuva_ops", but it returns an int.

Non-blocking nits:

1. The Rust formatting does not look rustfmt-clean (the fetch_update(...).map_err(...) chain in op_map, the four multi-line pin_init!(VmBoData { ... }) sites, the closure in lifetime.rs). I did not run rustfmt. If the tree checks rustfmtcheck, run it.
2. The same VmBoData { newest_bind: AtomicU64::new(0), segments <- new_mutex!(None) } initialiser is now written four times; a small VmBoData::init() would stop a fifth site from forgetting the field. The compiler would catch it, so this is only tidiness.
3. The filtered fallback walks the whole VM list under the VM lock. That is only reached when a reimport made newest_bind newer, which is rare, so I would not add anything; just worth knowing if a profile ever shows it.

Not checked: build, rustfmt, any run. The description reports 50/50 regression cases, 10 Settings launches and 10 Light/Dark cycles on a 14-inch M3 Pro; those do not deterministically hit "close, reimport, bind, drain" in that order, so a unit-level or loop test for that order would be the next thing worth having, but I do not hold the merge for it.

[chriskearney]

Reviewed 13c3371ce23dceabad0fffd1d16ee121adb93dff. No blocking finding in the changed code. The close-time bound is applied consistently to GPUVAs, context bindings and shared bindings; remap pieces retain their original sequence, and both ENOMEM paths use filtered cleanup. Initializing the empty BO-unmap result is also needed.

Built and booted that exact head on a 16-inch M3 Pro (J516S), M1 MacBook Pro (J293) and M1 Air (J313), with native Mesa 147a62f619 unchanged.

Independent shared-buffer stress on each Mac: 100/100 cycles passed, checking all 16,384 words after GPU readback each time. A separate DRM file retains the producer dma-buf; the Vulkan consumer closes and reimports it while unrelated valid GPU work is outstanding, waits for that work to retire, then fills and reads the imported buffer. The fence remained pending across close and reimport in every cycle. A libdrm observer confirmed actual GEM closes, PRIME imports and new VM binds. This exercises the relevant userspace traffic, but does not instrument the kernel's exact drain scheduling.

OpenGL and Vulkan rendered-pixel checks also passed on all three, with no GPU/DART faults observed during the test workloads. Connected external displays remained active: 4K/120 on J293, 1080p/60 on J313, and two 4K/60 outputs on J516S.

Machine	vkQuake demo1 median FPS	glmark2	vkmark
M1 MacBook Pro	512.0	1280	7592
M1 Air	510.0	1227	7328
M3 Pro	187.5	709	2272

vkQuake used the same executable/assets/settings, screenshot-confirmed 1920×1080, one warmup and three measured runs. Differences from our earlier kernels were small; these results do not establish a performance gain.

Limits: I have not independently reproduced the original Plasma crash on an unpatched kernel. Our desktop differs from stock KWin, and this is neither full CTS nor forced-ENOMEM coverage. This is not full hardware qualification. Both M1s returned to their normal kernels with original boot files/packages verified. The M3 return-boot failure was traced to our local cleanup service deleting the baseline kernel’s unowned modules during the test boot. This was a module-retention mistake outside this diff. The M3 is back on its preserved fallback; the deleted modules are restored and retained, but the newer baseline has not been boot-tested again.

[chriskearney]

Approved at 13c3371 after code review and hardware testing on J516S, J293 and J313. Detailed results and test limitations are in my earlier review; no blocking finding in this diff.

[joshuaswarren]

Hand build of integration/aurora tip 19f4965 (this PR, merged; also carries #211/#212/#213). Package release 7.2.11-tip19f49651-ARCH+, package sha256 36952afbd4b3f4270ccbb166857dab926c6abf56a0bb0bac185c3ab765c17d80, one-shot boot only, stock default untouched.

machine	boots	result
14-inch M2 Max	yes — candidate boot 3d9ed350, return boot 8035e6ae (12.6 default)	smoke equal to stock; m-test 0.1.13: https://omarchy-m-testing.org/reports/T5BU9yaPgbAzBFoAcPpFH14d

CI on this PR: 12 pass, 2 skipped.

Regressions: none found. Boot/wifi/input/display/audio/ANE all pass; dmesg error classes match the stock baseline (no new GPU/MMU/UAT/fault lines — the only grep hit is the benign "iommu: Default domain type" boot line); GPU driver, Vulkan and OpenGL checks pass; SEP stays healthy (no failure/refusal lines; this machine has an enrolled fingerprint, and the driver behaved with xart writes enabled). glmark2-es2 on the candidate: build 9352, shading 9031 — the stock numbers taken the same day (6513/5489) were measured at 18% battery while charging, so the pair is not a like-for-like comparison; treat it as "runs, no faults", not a perf claim.

Limit: this machine has an M2 GPU, so this PR's M3-Pro deferred-close path is not exercised — it proves the surrounding gpuvm/GEM changes compile, boot and do not regress the shared path on M2.