Claude review: RDMA/mlx5: get tph for p2p access when registering dma-buf mr

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Double `dma_buf_get` on the same fd**

`get_tph_mr_dmabuf()` calls `dma_buf_get(fd)` to temporarily resolve the fd just to query TPH metadata:

```c
dmabuf = dma_buf_get(fd);
if (IS_ERR(dmabuf))
    return;
...
dma_buf_put(dmabuf);
```

The same fd is resolved again later by the main MR registration path (`ib_umem_dmabuf_get_pinned` or similar). This is functionally correct but does an unnecessary extra get/put cycle. Consider whether the dma_buf pointer could be passed from the caller instead, though I understand it may not be available at this point in the flow.

**Direct callback invocation instead of helper**

The code calls `dmabuf->ops->get_tph()` directly:

```c
if (!dmabuf->ops->get_tph)
    goto end_dbuf_put;

ret = dmabuf->ops->get_tph(dmabuf, &steering_tag, ph, st_width);
```

This is the standard pattern for optional dma-buf callbacks (same as how `pin`/`unpin` are called). It would be worth considering whether a `dma_buf_get_tph()` inline helper in `<linux/dma-buf.h>` would be better for encapsulation, especially if other importers will use this callback. But for an initial implementation with a single consumer, this is fine.

**Error handling in `get_tph_mr_dmabuf` is correct**

If `mlx5_st_alloc_index_by_tag` fails, `*ph` is reset to `MLX5_IB_NO_PH`:

```c
ret = mlx5_st_alloc_index_by_tag(dev->mdev, steering_tag, st_index);
if (ret) {
    *ph = MLX5_IB_NO_PH;
    ...
}
```

`*st_index` was never modified by the failed call, so it retains the caller's default (`MLX5_MKC_PCIE_TPH_NO_STEERING_TAG_INDEX`). This is correct — a failed TPH query silently falls back to no-TPH behavior.

**`ret = 0` initialization in `mlx5_st_create` is a real fix**

Looking at the existing code, when the `xa_for_each` loop finds a cached tag match:

```c
xa_for_each(&st->idx_xa, index, idx_data) {
    if (tag == idx_data->tag) {
        refcount_inc(&idx_data->usecount);
        *st_index = index;
        goto end;
    }
}
...
end:
    mutex_unlock(&st->lock);
    return ret;  // <-- was uninitialized on cache hit path
```

`ret` is genuinely uninitialized on the cache-hit `goto end` path. The `ret = 0` init in the refactored `mlx5_st_alloc_index_by_tag` fixes this bug. This is correct and should be called out more prominently — it's a real bug fix, not just a clang warning suppression.

**Refactoring of `mlx5_st_alloc_index` is clean**

The split into `mlx5_st_alloc_index_by_tag` (takes raw tag) + `mlx5_st_alloc_index` (wraps with `pcie_tph_get_cpu_st`) is a straightforward factoring. The original function body moves to `_by_tag`, and the original becomes a thin wrapper. The `#else` stub for `!CONFIG_PCIE_TPH` is correctly added.

**`MODULE_IMPORT_NS("DMA_BUF")` is needed**

`dma_buf_get` is exported under the `DMA_BUF` namespace, so this is required.

---
Generated by Claude Code Patch Reviewer