On 2026-10-26 at 19:03 +1100, Karl Mehltretter <[email protected]> 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
