All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Romanovsky <leonro@mellanox.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Jason Gunthorpe <jgg@ziepe.ca>,
	Doug Ledford <dledford@redhat.com>,
	linux-rdma@vger.kernel.org,
	Laurence Oberman <loberman@redhat.com>,
	Kamal Heib <kamalheib1@gmail.com>
Subject: Re: [PATCH v2 4/4] RDMA/srpt: Increase max_send_sge
Date: Tue, 26 May 2020 08:55:23 +0300	[thread overview]
Message-ID: <20200526055523.GQ10591@unreal> (raw)
In-Reply-To: <0ca86214-372d-eefe-d259-d50d1e18f096@acm.org>

On Mon, May 25, 2020 at 02:51:25PM -0700, Bart Van Assche wrote:
> On 2020-05-25 12:15, Bart Van Assche wrote:
> > On 2020-05-25 10:51, Leon Romanovsky wrote:
> >> On Mon, May 25, 2020 at 10:22:12AM -0700, Bart Van Assche wrote:
> >>> The ib_srpt driver limits max_send_sge to 16. Since that is a workaround
> >>> for an mlx4 bug that has been fixed, increase max_send_sge. For mlx4, do
> >>> not use the value advertised by the driver (32) since that causes QP's
> >>> to transition to the error status.
> >>
> >> How are you avoiding mlx4 bug in this patch?
> >> Isn't "attrs->max_send_sge" come from driver as is?
> >
> > Hi Leon,
> >
> > Development of this patch started considerable time ago - before the
> > ib_srpt driver was converted to the RDMA R/W API. Before that conversion
> > the ib_srpt driver was using attrs->max_send_sge limit for both RDMA
> > writes and RDMA reads, which is wrong. Hence the need for the
> > "max_sge_delta" parameter (max_send_sge = 32 and max_sge_rd = 30 for
> > mlx4). The following code from drivers/infiniband/core/rw.c selects the
> > proper limit:
> >
> >        u32 max_sge = dir == DMA_TO_DEVICE ? qp->max_write_sge :
> >                      qp->max_read_sge;
> >
> > The following code in drivers/infiniband/core/verbs.c sets these limits:
> >
> >         qp->max_write_sge = qp_init_attr->cap.max_send_sge;
> >         qp->max_read_sge = min_t(u32, qp_init_attr->cap.max_send_sge,
> >                                  device->attrs.max_sge_rd);
>
> The commit message should be shortened to the following: "The ib_srpt
> driver limits max_send_sge to 16. Since that is a workaround
> for an mlx4 bug that has been fixed, increase max_send_sge. See also
> commit f95ccffc715b ("IB/mlx4: Use 4K pages for kernel QP's WQE buffer")."

Yes, please. The proposed commit message describes better the change.

Thanks

>
> Bart.

  reply	other threads:[~2020-05-26  5:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-25 17:22 [PATCH v2 0/4] Four SRP initiator and target patches Bart Van Assche
2020-05-25 17:22 ` [PATCH v2 1/4] RDMA/srp: Make the channel count configurable per target Bart Van Assche
2020-05-25 17:22 ` [PATCH v2 2/4] RDMA/srpt: Make debug output more detailed Bart Van Assche
2020-05-25 17:22 ` [PATCH v2 3/4] RDMA/srpt: Reduce max_recv_sge to 1 Bart Van Assche
2020-05-25 17:22 ` [PATCH v2 4/4] RDMA/srpt: Increase max_send_sge Bart Van Assche
2020-05-25 17:51   ` Leon Romanovsky
2020-05-25 19:15     ` Bart Van Assche
2020-05-25 21:51       ` Bart Van Assche
2020-05-26  5:55         ` Leon Romanovsky [this message]
2020-05-29 18:14 ` [PATCH v2 0/4] Four SRP initiator and target patches Jason Gunthorpe

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=20200526055523.GQ10591@unreal \
    --to=leonro@mellanox.com \
    --cc=bvanassche@acm.org \
    --cc=dledford@redhat.com \
    --cc=jgg@ziepe.ca \
    --cc=kamalheib1@gmail.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=loberman@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.