Re: [PATCH rdma-next v3 03/17] RDMA/core: Introduce generic buffer descriptor infrastructure for umem

[Jiri Pirko <jiri@resnulli.us>]

Thu, May 14, 2026 at 01:34:47AM +0200, jgg@ziepe.ca wrote:
>On Wed, May 13, 2026 at 09:18:28PM +0200, Jiri Pirko wrote:
>> Tue, May 12, 2026 at 08:12:36PM +0200, jgg@ziepe.ca wrote:
>> >On Wed, May 06, 2026 at 04:14:34PM +0200, Jiri Pirko wrote:
>> >> Wed, May 06, 2026 at 03:37:57PM +0200, jgg@ziepe.ca wrote:
>> >> >On Mon, May 04, 2026 at 03:57:17PM +0200, Jiri Pirko wrote:
>> >> >
>> >> >> +/**
>> >> >> + * ib_umem_get - Canonical on-demand umem getter.
>> >> >> + * @device:        IB device.
>> >> >> + * @udata:         uverbs udata bundle (may be NULL).
>> >> >> + * @attr_id:       per-command UMEM attribute id; consulted if @udata is set.
>> >> >> + * @legacy_filler: optional command-specific legacy attr filler.
>> >> >> + *                 invoked if @udata is set.
>> >> >> + * @va_fallback:   if true, build a VA-typed desc with @addr.
>> >> >> + * @addr:          user VA, used if @va_fallback is true.
>> >> >> + * @size:          driver-required minimum length.
>> >> >> + * @access:        IB access flags forwarded to ib_umem_get_desc().
>> >> >> + *
>> >> >> + * Return: valid umem on success, ERR_PTR(...) on error, NULL
>> >> >> + * if no source produced a buffer (only possible when @va_fallback is false).
>> >> >> + */
>> >> >> +struct ib_umem *ib_umem_get(struct ib_device *device, struct ib_udata *udata,
>> >> >> +			    u16 attr_id,
>> >> >> +			    ib_umem_buf_desc_filler_t legacy_filler,
>> >> >> +			    bool va_fallback, u64 addr, size_t size, int access)
>> >> >
>> >> >I didn't try to look at what the drivers actually do, but I'm slightly
>> >> >surprised not to see an addr_size here? Is it the case the drivers
>> >> >don't have have a uhw->size to go along with their uhw->va?
>> >> 
>> >> "size_t size". What am I missing?
>> >
>> >size is the minimum length, not the actual length passed into the
>> >system call
>> 
>> Right, you're correct. Size is the post-pin minimum, and on
>> the VA fallback path it serves also as the pin length (we synthesize
>> desc.length = size when no attr/legacy filler matched).
>> 
>> There's no addr_size because no caller has a user-passed
>> length distinct from the driver-required minimum.
>> Drivers either have no length in their legacy ucmd (mlx4/mlx5 CQ, mlx5 QP,
>> the size is driver-computed from entries*cqe_size etc.) or they
>> pass ucmd.buf_size which serves as both (vmw_pvrdma, qedr, mlx5
>> SRQ, ...).
>
>This is what I was wondering about, so how does it work when there is
>a ucmd.buf_size and also a minimum size computed from
>entries*cqe_size? What does the driver write?
>
>I think the driver has to pass in both the minimum size computed from
>entries*cqe_size and also the uhw exact size? The minimum size is used
>if the uhw path isn't used to check the other attribute while the uhw
>exact size is used to pin the memory if the uhw path is used - and it
>should also be checked against the minimum?

I asked Claude to check every caller: At the moment of conversion
none of them would have addr_size != min_size. They split into
three groups:

  1) Driver-computed total only (no user-passed length distinct from
     min). addr_size == min_size by construction:

       mlx4 CQ/QP/SRQ           entries * cqe_size, qp->buf_size, ...
       mlx5 CQ/QP/SRQ           entries * cqe_size, rwq->buf_size, ...
       bnxt_re QP/SRQ/CQ-resize max_wqe * wqe_size, entries * sizeof
       mana CQ                  cq->cqe * COMP_ENTRY_SIZE
       hns_roce MTR             mtr_bufs_size(buf_attr)
       qedr SRQ producer pair   sizeof(struct rdma_srq_producers)
       all DBR helpers          PAGE_SIZE

  2) MR registration / opaque user umem. The user-passed length *is*
     the request; there's no separate driver minimum. addr_size ==
     min_size by definition:

       reg_user_mr in every driver
       mlx5 devx user umem

  3) User-passed length without a driver minimum cross-check today:

       vmw_pvrdma CQ/QP/SRQ     derivable min (entries*sizeof, wqe_*),
                                not computed in the user path
       qedr CQ/QP/SRQ           ureq.size, no min computed in wrapper
       mana WQ, QP raw_sq,
            QP RC queues        ucmd.{wq,sq}_buf_size, queue_size[],
                                no derivable min
       ionic CQ, QP sq/rq       req_cq->size, sq->size, rq->size,
                                no derivable min

For completeness, the two drivers that don't go through ib_umem use
the same single-length shape:

       hfi1   hfi1_acquire_user_pages(vaddr, npages, ...) — thin wrapper
              around pin_user_pages_fast(); no separate driver-min check.
       usnic  usnic_uiom_get_pages(addr, size, ...) — single size, same
              shape as ib_umem_get_va().

So across the entire RDMA tree (umem and non-umem) every user-memory
pin helper currently takes a single length, and no caller
distinguishes a user-passed addr_size from a driver-computed min_size.

Group 3 is where addr_size != min_size could become real, but only
after each driver is independently tightened. Until then the split
just makes every caller pass the same value twice and would leave
ib_umem_get() the only helper in the subsystem that enforces the
distinction structurally.

Given that, I'd prefer to keep the single size argument for now and
spell the contract out in the kdoc:

  @size: minimum required umem length, validated post-pin against any
         descriptor produced via @attr_id / @legacy_filler; also used
         as the pin length on the VA fallback path. Callers that have
         a distinct user-passed length must validate it against their
         driver minimum before calling.

If/when a driver actually needs distinct values, splitting into
addr_size + min_size is mechanical.

Would that work for you?