Re: [PATCH rdma-next 01/10] RDMA/umem: Add reference counting to ib_umem

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Feb 03, 2026 at 04:39:52PM +0100, Jiri Pirko wrote:
> Tue, Feb 03, 2026 at 03:51:38PM +0100, jgg@ziepe.ca wrote:
> >On Tue, Feb 03, 2026 at 09:49:53AM +0100, Jiri Pirko wrote:
> >> From: Jiri Pirko <jiri@nvidia.com>
> >> 
> >> Introduce reference counting for ib_umem objects to simplify memory
> >> lifecycle management when umem buffers are shared between the core
> >> layer and device drivers.
> >
> >I admit I have reservations about this too.. The flow should not be so
> >convoluted that a refcount is necessary. The lifecycle of a umem is
> >not uncertain at all.
> >
> >I imagine'd it would be like:
> >
> >core code:
> >  if (ops->create_cq_umem) {
> >     umem = umem_get
> >     rc = ops->create_cq_umem(umem)
> >     if (rc)
> >      umem_free(umem)
> >  } else {
> >     rc = ops->create_cq()
> >  }
> >
> >Driver:
> >  create_cq():
> >    copy_from_user(drvdata)
> >    umem = umem_get()
> >    rc = driver_create_cq_umem(umem, &drvdata))
> >    if (rc)
> >      umem_free(umem)
> >
> >   create_cq_umem()
> >     copy_from_user(drvdata)
> >     return driver_create_cq_umem(umem, &drvdata)
> >
> >   destroy_cq()
> >     destry_hw
> >     umem_free()
> 
> 
> This is how it is now. However there are couple of challenges about this
> flow:
> 1) umem usage. For example, create_qp_umem at the end of the set gets 4
>    umem pointers. sq,rq,sq_dbr,rq_dbr. Some driver may use only one of
>    those, 2 of those, 3 of those. Depends. mlx5 actually uses 2 or 3.
>    If what you suggest, the current approach stands, the user has to
>    always take all pointers, store them and eventually release them on
>    destroy_qp path.

Userspace passing umems that are not used by the driver is an error.
Fail the call.

> 2) error path. I found the error path quite odd. Then create_cq/qp_umem
>    returns !=0, core releases all umems. However, standard cq/qp
>    destroy path takes care of releasing umems. Since a lot of code on
>    error path and destroy path is shared, it has to be informed to
>    release or not release the umems. That is not nice.

Generally I would not assign to the driver's umem storage until the
creation is completed to avoid this. ie it stays null until committed.

But looking at mlx5 that looks like quite a maze there.. Yikes..

So maybe mlx5 adds some NULL assignments on its error paths and less
convoluted drivers can use a simpler option?

My issue with refcounts is that this isn't a refcounted structure, it
has very well defined points where it must become freed.

Like we can't get through detroy_qp without the umem being freed, that
is illegal.

Jason