On 11/9/26 03:58, Alistair Popple wrote:
On 2026-11-07 at 19:57 +1100, Karl Mehltretter kmehltretter@gmail.com wrote...
On Thu, Nov 05, 2026 at 05:40:41PM +0100, Alistair Popple wrote:
- A pin or map that fails after dma_buf_mapping_select() may reopen the
- choice only if it was the one that made it. A selection that predates the
- call belongs to a live pin or mapping and must survive, otherwise the next
- pin or map would reselect under it without an invalidate.
- */
+void dma_buf_mapping_unselect(struct dma_buf_attachment *attach,
bool was_selected)+{
- dma_resv_assert_held(attach->dmabuf->resv);
- if (was_selected)
return;- attach->selected = false;
- attach->selected_idx = 0;
+}
After dma_buf_invalidate_mappings(), attach->selected is false while attach->selected_idx still holds that mapping's entry for a pending unmap.
If a pin/map fails before that unmap runs, was_selected is false and dma_buf_mapping_unselect() resets attach->selected_idx to 0.
A subsequent dma_buf_unmap_sgt() then uses matches[0].
Actually there's a bigger issue here. The contract at the moment states that mapping type may change after invalidation. However that doesn't account for the fact mappings can outlive the invalidation.
This could be dealt with by switching everything over to the generic dma_buf_mapping_map/unmap() path, but that requires an allocation per mapping just to track the mapping type.
You need an allocation per mapping anyway to identify the mapping anyway.
We currently abuse the sg_table for that, but essentially the idea of having a structure/object to represent the mapping needs to stay around as far as I can see.
Regards, Christian.
The alternate is we update the contract to say mapping type cannot change until after all importers have unmapped. Enforcing this would require tracking the number of live mappings/pins per-attachment, but I think this is the better solution so will add that for the next version.
int dma_buf_pin(struct dma_buf_attachment *attach) { struct dma_buf *dmabuf = attach->dmabuf;
- bool selected = attach->selected; int ret = 0;
WARN_ON(!attach->importer_ops); dma_resv_assert_held(dmabuf->resv);
- if (dmabuf->ops->pin)
- ret = dma_buf_mapping_select(attach);
- if (ret)
return ret;- if (dmabuf->ops->pin) { ret = dmabuf->ops->pin(attach);
if (ret)dma_buf_mapping_unselect(attach, selected);- }
dma_buf_mapping_select() sets attach->selected = true before ops->pin() and the map callbacks run.
In patches 14/15, xe/amdgpu migrate a VRAM buffer to system memory during pin and map when the attachment does not accept P2P.
That migration calls dma_buf_invalidate_mappings() and clears attach->selected on every attachment, also the one being pinned/mapped.
When the callback succeeds, the attachment stays pinned or mapped with attach->selected == false.
A later map/pin on the same attachment can then reselect without an invalidation.
Argh. Thanks. Will fix.
- Alistair
Thanks, Karl