Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] RDMA/uverbs: commit message contradicts uverbs_dmabuf_match_mapping() --
commit f919774da72af458231453cd6f22c8076d245156 Author: Jason Gunthorpe <[email protected]> RDMA/uverbs: Use the SGT_P2P mapping type This commit converts uverbs dmabuf from an attach callback checking attachment->peer2peer to a match_mapping callback using DMA_BUF_EMAPPING_SGT_REQUIRES_P2P(). > RDMA/uverbs: Use the SGT_P2P mapping type > > Very similar to habana, except the rejection of unsupported > pci_p2p_dma_distance() is done inside map not attach via > dma_buf_phys_vec_to_sgt(). > > Convert this directly to a SGT_P2P match which will prevent matching if > dma_buf_phys_vec_to_sgt() would fail. > > Remove the now empty attach function. > > Signed-off-by: Jason Gunthorpe <[email protected]> > Signed-off-by: Alistair Popple <[email protected]> > > diff --git a/drivers/infiniband/core/uverbs_std_types_dmabuf.c > b/drivers/infiniband/core/uverbs_std_types_dmabuf.c > index 8d8c997a48834..0bae578c3d78e 100644 > --- a/drivers/infiniband/core/uverbs_std_types_dmabuf.c > +++ b/drivers/infiniband/core/uverbs_std_types_dmabuf.c [ ... ] > @@ -95,13 +86,30 @@ static void uverbs_dmabuf_release(struct dma_buf *dmabuf) > uverbs_uobject_put(&priv->uobj); > } > > -static const struct dma_buf_ops uverbs_dmabuf_ops = { > - .attach = uverbs_dmabuf_attach, > +static const struct dma_buf_mapping_sgt_exp_ops uverbs_dma_buf_sgt_ops = { > .map_dma_buf = uverbs_dmabuf_map, > .unmap_dma_buf = uverbs_dmabuf_unmap, > +}; > + > +static int uverbs_dmabuf_match_mapping(struct dma_buf_match_args *args) > +{ > + struct dma_buf_mapping_match sgt_match[1]; > + > + /* > + * MMIO, so the importer must accept P2P. The provider is only valid > + * under the dma_resv lock while not revoked, so the distance check is > + * left to dma_buf_phys_vec_to_sgt() at map time, as before. > + */ > + sgt_match[0] = > DMA_BUF_EMAPPING_SGT_REQUIRES_P2P(&uverbs_dma_buf_sgt_ops); [Severity: Medium] Does the commit message match the implementation in uverbs_dmabuf_match_mapping()? The commit title and changelog state that this converts uverbs to a SGT_P2P match which prevents matching if dma_buf_phys_vec_to_sgt() would fail due to an unsupported pci_p2p_dma_distance(). However, uverbs_dmabuf_match_mapping() uses DMA_BUF_EMAPPING_SGT_REQUIRES_P2P() rather than DMA_BUF_EMAPPING_SGT_P2P(). As noted in the code comment, DMA_BUF_EMAPPING_SGT_REQUIRES_P2P() only checks that the importer accepts P2P (replacing the !attachment->peer2peer check in uverbs_dmabuf_attach()) and leaves the P2P distance check to dma_buf_phys_vec_to_sgt() at map time because priv->provider is only valid under the dma_resv lock while not revoked. Should the commit title and message be updated to reflect that DMA_BUF_EMAPPING_SGT_REQUIRES_P2P() is used instead of SGT_P2P, and that the distance check remains at map time? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=17
