public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Edward Srouji <edwards@nvidia.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Chuck Lever <cel@kernel.org>,
	Christoph Hellwig <hch@infradead.org>,
	NeilBrown <neil@brown.name>, Jeff Layton <jlayton@kernel.org>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	Anna Schumaker <anna@kernel.org>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
	Chuck Lever <chuck.lever@oracle.com>,
	Leon Romanovsky <leon@kernel.org>
Subject: Re: [PATCH v4 01/14] svcrdma: Reduce the number of rdma_rw contexts per-QP
Date: Sun, 11 May 2025 02:12:16 +0300	[thread overview]
Message-ID: <e9e3c6ba-3a45-4fe6-967a-ffa100705663@nvidia.com> (raw)
In-Reply-To: <20250508124328.GA6500@ziepe.ca>


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

  reply	other threads:[~2025-05-10 23:12 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-28 19:36 [PATCH v4 00/14] Allocate payload arrays dynamically cel
2025-04-28 19:36 ` [PATCH v4 01/14] svcrdma: Reduce the number of rdma_rw contexts per-QP cel
2025-05-06 13:08   ` Christoph Hellwig
2025-05-06 13:17     ` Jason Gunthorpe
2025-05-06 13:40       ` Christoph Hellwig
2025-05-06 13:55         ` Jason Gunthorpe
2025-05-06 14:13           ` Chuck Lever
2025-05-06 14:17             ` Jason Gunthorpe
2025-05-06 14:19               ` Chuck Lever
2025-05-06 14:22                 ` Jason Gunthorpe
2025-05-08  8:41                   ` Edward Srouji
2025-05-08 12:43                     ` Jason Gunthorpe
2025-05-10 23:12                       ` Edward Srouji [this message]
2025-04-28 19:36 ` [PATCH v4 02/14] sunrpc: Add a helper to derive maxpages from sv_max_mesg cel
2025-05-06 13:10   ` Christoph Hellwig
2025-04-28 19:36 ` [PATCH v4 03/14] sunrpc: Remove backchannel check in svc_init_buffer() cel
2025-05-06 13:11   ` Christoph Hellwig
2025-04-28 19:36 ` [PATCH v4 04/14] sunrpc: Replace the rq_pages array with dynamically-allocated memory cel
2025-04-30  4:53   ` NeilBrown
2025-04-28 19:36 ` [PATCH v4 05/14] sunrpc: Replace the rq_vec " cel
2025-05-06 13:29   ` Christoph Hellwig
2025-05-06 16:31     ` Chuck Lever
2025-05-07  7:34       ` Christoph Hellwig
2025-04-28 19:36 ` [PATCH v4 06/14] sunrpc: Replace the rq_bvec " cel
2025-04-28 19:36 ` [PATCH v4 07/14] sunrpc: Adjust size of socket's receive page array dynamically cel
2025-04-28 19:36 ` [PATCH v4 08/14] svcrdma: Adjust the number of entries in svc_rdma_recv_ctxt::rc_pages cel
2025-05-06 13:31   ` Christoph Hellwig
2025-05-06 15:20     ` Chuck Lever
2025-05-07  7:40       ` Christoph Hellwig
2025-04-28 19:36 ` [PATCH v4 09/14] svcrdma: Adjust the number of entries in svc_rdma_send_ctxt::sc_pages cel
2025-04-28 19:36 ` [PATCH v4 10/14] sunrpc: Remove the RPCSVC_MAXPAGES macro cel
2025-04-28 19:36 ` [PATCH v4 11/14] NFSD: Remove NFSD_BUFSIZE cel
2025-04-28 21:03   ` Jeff Layton
2025-05-06 13:32   ` Christoph Hellwig
2025-04-28 19:37 ` [PATCH v4 12/14] NFSD: Remove NFSSVC_MAXBLKSIZE_V2 macro cel
2025-05-06 13:33   ` Christoph Hellwig
2025-04-28 19:37 ` [PATCH v4 13/14] NFSD: Add a "default" block size cel
2025-04-28 21:07   ` Jeff Layton
2025-04-28 19:37 ` [PATCH v4 14/14] SUNRPC: Bump the maximum payload size for the server cel
2025-04-28 21:08   ` Jeff Layton
2025-04-29 15:44     ` Chuck Lever
2025-05-06 13:34   ` Christoph Hellwig
2025-05-06 13:52     ` Chuck Lever
2025-05-06 13:54       ` Jeff Layton
2025-05-06 13:59         ` Chuck Lever
2025-05-07  7:42       ` Christoph Hellwig
2025-05-07 14:25         ` Chuck Lever
2025-04-29 13:06 ` [PATCH v4 00/14] Allocate payload arrays dynamically Zhu Yanjun
2025-04-29 13:41   ` Chuck Lever
2025-04-29 13:52     ` Zhu Yanjun
2025-04-30  5:11 ` NeilBrown
2025-04-30 12:45   ` Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e9e3c6ba-3a45-4fe6-967a-ffa100705663@nvidia.com \
    --to=edwards@nvidia.com \
    --cc=anna@kernel.org \
    --cc=cel@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=dai.ngo@oracle.com \
    --cc=hch@infradead.org \
    --cc=jgg@ziepe.ca \
    --cc=jlayton@kernel.org \
    --cc=leon@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox