Re: [PATCH v4 01/14] svcrdma: Reduce the number of rdma_rw contexts per-QP

Edward Srouji
2025-05-10T23:12:28Z

On 5/8/2025 3:43 PM, Jason Gunthorpe wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, May 08, 2025 at 11:41:18AM +0300, Edward Srouji wrote:
>> On 5/6/2025 5:22 PM, Jason Gunthorpe wrote:
>>> On Tue, May 06, 2025 at 10:19:06AM -0400, Chuck Lever wrote:
>>>>>> In this patch I'm trying to include the reg/inv multiplier in the
>>>>>> calculation, but that doesn't seem to be enough to make "accept"
>>>>>> reliable, IMO due to this extra calculation in calc_sq_size().
>>>>> Did ib_create_qp get called with more than max_qp_wr ?
>>>> The request was for, like, 9300 SQEs. max_qp_wr is 32K on my systems.
>>> Sounds like it is broken then..
>>>
>>> props->max_qp_wr = 1 << MLX5_CAP_GEN(mdev, log_max_qp_sz);
>>>
>>> So it is ignoring the wqe_size adustment.. It should adjust by the worst
>>> case result of calc_send_wqe() for the device..
>> How do you suggest adjusting to the worst case?
>> How inline messages could be addressed and taken into account?
> I think assume 0 size inline for computing max sizes
>
>> Even if we ignore the inline size, worst case potentially could be less than
>> 1/8th of the max HCA CAP, not sure we want to deliver this as a limitation
>> to users.
> The math is simply wrong - log_max_qp_sz is not the number of work
> queue entries in the queue, it is the number of MLX5_SEND_WQE_BB's
> units which is some internal value.

I agree, no doubt that the returned max_qp_wr is wrong and misleading...

>
> For a verbs API the result should be the max number of work queue
> entries that can be requested for any of XRC/RC/UC/UD QP types using a
> 0 inline size, 1 SGL and no other special features.
>
> Even for a simple RC QP sq_overhead() will return 132 which already
> makes props->max_qp_wr uselessly wrong. 132 goes into here:
>
> return ALIGN(max_t(int, inl_size, size), MLX5_SEND_WQE_BB);
>
> Comes out as 192 - so props->max_qp_wr is off by 3x even for a simple
> no-feature RC QP.
>
> Chuck is getting:
>
> calc_sq_size:618:(pid 1514): send queue size (9326 * 256 / 64 -> 65536) exceeds limits(32768)
>
> So I suppose that extra 64 bytes is coming from cap.max_send_sge >= 3?
>
> Without a new API we can't make it fully discoverable, but the way it
> is now is clearly wrong.

This is what I was considering, a new API.
The above suggested calculation will return a reasonable value but
obviously won't satisfy all use-cases (probably someone else will face
similar issue later on).
The question is whether it is worth a new dedicated API for an accurate
"per user-case" calculation.

>
> Jason