Re: [PATCH rdma-next v2 01/15] RDMA/core: Introduce generic buffer descriptor infrastructure for umem

[Jason Gunthorpe <jgg@ziepe.ca>]

On Sat, Apr 11, 2026 at 04:49:01PM +0200, Jiri Pirko wrote:
> @@ -332,3 +333,250 @@ int ib_umem_copy_from(void *dst, struct ib_umem *umem, size_t offset,

> +struct ib_umem_list *ib_umem_list_create(struct ib_device *device,
> +					 const struct uverbs_attr_bundle *attrs,
> +					 unsigned int slot_max)
> +{
> +	const struct ib_uverbs_buffer_desc *descs;
> +	struct ib_umem_dmabuf *umem_dmabuf;
> +	struct ib_umem_list *list;
> +	struct ib_umem *umem;
> +	unsigned int count;
> +	int num_descs;
> +	int err;
> +	int i;
> +
> +	if (WARN_ON_ONCE(slot_max >= BITS_PER_LONG))
> +		return ERR_PTR(-EINVAL);
> +	count = slot_max + 1;
> +
> +	num_descs = uverbs_attr_ptr_get_array_size(
> +		(struct uverbs_attr_bundle *)attrs, UVERBS_ATTR_BUFFERS,
> +		sizeof(*descs));

uverbs_attr_ptr_get_array_size() should get a const on the parameter,
seems to have been missed originally

> +/*
> + * Describes a single buffer backed by dma-buf or user virtual address.
> + * Passed as an array via UVERBS_ATTR_BUFFERS. Each uverb command that
> + * accepts this attribute defines its own per-command buffer slot enum.
> + * The index field selects the buffer slot this descriptor maps to.
> + *
> + * @fd: dma-buf file descriptor (valid for IB_UVERBS_BUFFER_TYPE_DMABUF)
> + * @type: buffer type from enum ib_uverbs_buffer_type
> + * @index: per-command buffer slot index
> + * @reserved: must be zero
> + * @addr: offset within dma-buf, or user virtual address for VA
> + * @length: buffer length in bytes
> + */
> +struct ib_uverbs_buffer_desc {
> +	__s32 fd;
> +	__u32 type;
> +	__u32 index;
> +	__u32 reserved;
> +	__aligned_u64 addr;
> +	__aligned_u64 length;
> +};

This seems like a good idea, we should have done it earlier :\

Arguably if you do this then the first issue of being more flexible
with umems is addressed, so a uverbs_attr_ptr_get_umem() looks much
more feasible.

Just brain storming, but if we let the driver pass in its uhw
information inot a getter:

  struct ib_umem *uverbs_attr_get_umem(struct
      uverbs_attr_bundle *attrs, u16 idx,
      u64 uhw_umem_base, u64 umem_len);

  dbr_umem = uverbs_attr_get_umem(attrs,
                     MLX5_IB_ATTR_QP_DBR, uhw->base, uhw->len);

Then if the new attribute is provided the uhw is ignored, otherwise a
ib_uverbs_buffer_desc is created from the udata parameters instead.

Drivers use the normal attr indexes to define their many umems for
something complicated lik QP.

For the lifecycle.. This series adds a 
  +       cq->umem_list     = umem_list;
 
So it is not a big leap to imagine a linked list in the object that is
appended by the umem create function. Pass the list head into the umem
allocator, free the whole linked list in the core code.

This has some appeal because it is an easier conversion of all the
drivers, instead of re-threading their flows to accept a pre-created
umem they just have to be updated to call the new function in all the
places they are currently getting umems.

You'd probably have a further helper for cq that could extract the
existing common cq attrs to a ib_uverbs_buffer_desc:

  cq_umem = uverbs_attr_get_cq_umem(attrs, cq, uhw->base, uhw->len);

Probably similar for mr and a common mr attribute.

This will be easier to put revocable and dynamic ops into the scheme,
they can be passed as arugments to the get function instead of some
complicated thing in the central ops structure.

Jason