Re: [PATCH 05/26] dma-buf: Let the exporter select mapping type at pin/map

[Alistair Popple <apopple@nvidia.com>]

On 2026-10-26 at 19:03 +1100, Karl Mehltretter <kmehltretter@gmail.com> wrote...
> On Mon, Oct 26, 2026 at 04:35:56PM +0100, Alistair Popple wrote:
> > +struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
> > +					enum dma_data_direction direction)
> > +{
> > +	struct sg_table *sg_table;
> > +	int ret;
> > +
> > +	might_sleep();
> > +
> > +	if (WARN_ON(!attach || !attach->dmabuf))
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	dma_resv_assert_held(attach->dmabuf->resv);
> > +
> > +	ret = dma_buf_mapping_select(attach);
> > +	if (ret)
> > +		return ERR_PTR(ret);
> > +
> > +	sg_table = __dma_buf_sgt_map_attachment(attach, direction);
> > +	if (IS_ERR(sg_table))
> > +		/* Nothing was established, so the choice may be made afresh */
> > +		attach->selected = false;
> > +
> > +	return sg_table;
> > +}
> 
> The map-failure report from Sashiko [1] reproduces with a KUnit test
> against the full series:
> 
>   pin selects B -> map returns -EIO -> selected is cleared
>                 -> retry selects A, with the original pin still held
> 
> The exporter pin count stays at 1. Preserving selected when it was true
> on entry makes the test pass. Patch 7 needs the same fix.

Agreed, thanks for looking. A failed map shouldn't undo a previous selection.

> Could dma_buf_mapping_select() report whether it made a new selection?
> Then the error path could undo only that choice, without dropping one
> established by an earlier pin or map.

We could do. At one stage I had a prototype of this returning the old match, but
that got complicated. We do need to undo in the error path though, so will fix
this for v2.

> On the Sashiko failed-pin report:
> ("Because the attachment is not pinned, the exporter may migrate the
> buffer elsewhere without invalidating it")
> 
> I don't think this follows from the documented contract.
> select_mapping() requires the exporter to keep the recorded type
> servable and invalidate before an incompatible move. That obligation
> still applies if ops->pin() fails.

Yes, I think that statement is correct.

> Is retaining the selection after a failed pin intentional? If so, this
> can be added as a test documenting that alongside the map-failure
> regression test.

No, that was not intentional. As noted about I think if the first pin/map of an
attachment failed it should effectively go back to the selected = false state.

 - Alistair

> Thanks,
> Karl