Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] Failure to re-query PCIe TPH metadata upon DMA-buf invalidation Pre-existing issues: - [High] Memory leak of struct mlx5_st_idx_data in mlx5_st_dealloc_index() -- commit e39795c3bd224c81bae70b87815075c4489e3f9e Author: Zhiping Zhang <[email protected]> Subject: RDMA/mlx5: get tph for p2p access when registering dma-buf mr This commit allows the mlx5 importer to query a dma-buf exporter's chosen PCIe TLP Processing Hint (TPH) Steering Tag. It updates MR registration to use this metadata for p2p access and program requester-side TPH on the outbound mkey, falling back to no-TPH routing if unavailable. > diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c > index e6b74955d95dc..55216ae637614 100644 > --- a/drivers/infiniband/hw/mlx5/mr.c > +++ b/drivers/infiniband/hw/mlx5/mr.c [ ... ] > @@ -903,6 +947,52 @@ static struct dma_buf_attach_ops > mlx5_ib_dmabuf_attach_ops = { > .invalidate_mappings = mlx5_ib_dmabuf_invalidate_cb, > }; > > +static void get_pci_tph_mr_dmabuf(struct mlx5_ib_dev *dev, struct dma_buf > *dmabuf, > + u16 *st_index, u8 *ph) [Severity: Medium] When an unpinned DMA-buf MR is invalidated and then subsequently faulted back in via pagefault_dmabuf_mr() in drivers/infiniband/hw/mlx5/odp.c, the driver calls ib_umem_dmabuf_map_pages() to map the pages, but it doesn't seem to re-query the TPH metadata using get_pci_tph_mr_dmabuf() or dma_buf_get_pci_tph(). Since the exporter might move the memory and change the preferred TPH steering tag, does the driver need to re-evaluate the TPH metadata upon invalidation and re-mapping to ensure DMA traffic isn't sent to a stale or suboptimal NUMA node? > +{ > + u16 local_st_index; > + u16 steering_tag; [ ... ] > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c > b/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c > index 997be91f0a135..bf14a7a6df15e 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c [ ... ] > @@ -152,8 +147,46 @@ int mlx5_st_alloc_index(struct mlx5_core_dev *dev, enum > tph_mem_type mem_type, > mutex_unlock(&st->lock); > return ret; > } > +EXPORT_SYMBOL_GPL(mlx5_st_alloc_index_by_tag); [ ... ] > +EXPORT_SYMBOL_GPL(mlx5_st_get_index); > + > int mlx5_st_dealloc_index(struct mlx5_core_dev *dev, u16 st_index) > { > struct mlx5_st_idx_data *idx_data; [Severity: High] This is a pre-existing issue, but I noticed it while reviewing this patch since this patch broadens its exposure. In mlx5_st_dealloc_index(), when the use count reaches zero: if (refcount_dec_and_test(&idx_data->usecount)) { xa_erase(&st->idx_xa, st_index); /* We leave PCI config space as was before, no mkey will refer to it */ } Is there a memory leak here? The dynamically allocated idx_data is erased from the xarray but does not appear to be passed to kfree(). Since unprivileged users can repeatedly create and destroy MRs, could this lead to kernel memory exhaustion over time? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
