* [PATCH 0/4] SRP initiator and target patches for kernel v5.8
@ 2020-05-22 21:33 Bart Van Assche
2020-05-22 21:33 ` [PATCH 1/4] RDMA/srp: Make the channel count configurable per target Bart Van Assche
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Bart Van Assche @ 2020-05-22 21:33 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Leon Romanovsky, Doug Ledford, linux-rdma, Bart Van Assche
Hi Jason,
Please consider these four patches for kernel v5.8.
Thanks,
Bart.
Bart Van Assche (4):
RDMA/srp: Make the channel count configurable per target
RDMA/srpt: Make debug output more detailed
RDMA/srpt: Reduce max_recv_sge to 1
RDMA/srpt: Increase max_send_sge
drivers/infiniband/ulp/srp/ib_srp.c | 21 ++++++++++---
drivers/infiniband/ulp/srpt/ib_srpt.c | 44 +++++++++++++++++++--------
drivers/infiniband/ulp/srpt/ib_srpt.h | 5 ---
3 files changed, 47 insertions(+), 23 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/4] RDMA/srp: Make the channel count configurable per target
2020-05-22 21:33 [PATCH 0/4] SRP initiator and target patches for kernel v5.8 Bart Van Assche
@ 2020-05-22 21:33 ` Bart Van Assche
2020-05-23 13:55 ` Leon Romanovsky
2020-05-22 21:33 ` [PATCH 2/4] RDMA/srpt: Make debug output more detailed Bart Van Assche
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2020-05-22 21:33 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Leon Romanovsky, Doug Ledford, linux-rdma, Bart Van Assche,
Laurence Oberman, Kamal Heib
Increase the flexibility of the SRP initiator driver by making the channel
count configurable per target instead of only providing a kernel module
parameter for configuring the channel count.
Cc: Laurence Oberman <loberman@redhat.com>
Cc: Kamal Heib <kamalheib1@gmail.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 00b4f88b113e..d686c39710c0 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3424,6 +3424,7 @@ enum {
SRP_OPT_IP_DEST = 1 << 16,
SRP_OPT_TARGET_CAN_QUEUE= 1 << 17,
SRP_OPT_MAX_IT_IU_SIZE = 1 << 18,
+ SRP_OPT_CH_COUNT = 1 << 19,
};
static unsigned int srp_opt_mandatory[] = {
@@ -3457,6 +3458,7 @@ static const match_table_t srp_opt_tokens = {
{ SRP_OPT_IP_SRC, "src=%s" },
{ SRP_OPT_IP_DEST, "dest=%s" },
{ SRP_OPT_MAX_IT_IU_SIZE, "max_it_iu_size=%d" },
+ { SRP_OPT_CH_COUNT, "ch_count=%d", },
{ SRP_OPT_ERR, NULL }
};
@@ -3758,6 +3760,14 @@ static int srp_parse_options(struct net *net, const char *buf,
target->max_it_iu_size = token;
break;
+ case SRP_OPT_CH_COUNT:
+ if (match_int(args, &token) || token < 1) {
+ pr_warn("bad channel count %s\n", p);
+ goto out;
+ }
+ target->ch_count = token;
+ break;
+
default:
pr_warn("unknown parameter or missing value '%s' in target creation request\n",
p);
@@ -3921,11 +3931,12 @@ static ssize_t srp_create_target(struct device *dev,
goto out;
ret = -ENOMEM;
- target->ch_count = max_t(unsigned, num_online_nodes(),
- min(ch_count ? :
- min(4 * num_online_nodes(),
- ibdev->num_comp_vectors),
- num_online_cpus()));
+ if (target->ch_count == 0)
+ target->ch_count = max_t(unsigned, num_online_nodes(),
+ min(ch_count ? :
+ min(4 * num_online_nodes(),
+ ibdev->num_comp_vectors),
+ num_online_cpus()));
target->ch = kcalloc(target->ch_count, sizeof(*target->ch),
GFP_KERNEL);
if (!target->ch)
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] RDMA/srpt: Make debug output more detailed
2020-05-22 21:33 [PATCH 0/4] SRP initiator and target patches for kernel v5.8 Bart Van Assche
2020-05-22 21:33 ` [PATCH 1/4] RDMA/srp: Make the channel count configurable per target Bart Van Assche
@ 2020-05-22 21:33 ` Bart Van Assche
2020-05-22 21:33 ` [PATCH 3/4] RDMA/srpt: Reduce max_recv_sge to 1 Bart Van Assche
2020-05-22 21:33 ` [PATCH 4/4] RDMA/srpt: Increase max_send_sge Bart Van Assche
3 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2020-05-22 21:33 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Leon Romanovsky, Doug Ledford, linux-rdma, Bart Van Assche,
Laurence Oberman, Kamal Heib
Since the session name by itself is not sufficient to uniquely identify a
queue pair, include the queue pair number. Show the ASCII channel state
name instead of the numeric value. This change makes the ib_srpt debug
output more consistent.
Cc: Laurence Oberman <loberman@redhat.com>
Cc: Kamal Heib <kamalheib1@gmail.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/infiniband/ulp/srpt/ib_srpt.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index a294630f2100..a39ad9fc4224 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -214,8 +214,9 @@ static const char *get_ch_state_name(enum rdma_ch_state s)
*/
static void srpt_qp_event(struct ib_event *event, struct srpt_rdma_ch *ch)
{
- pr_debug("QP event %d on ch=%p sess_name=%s state=%d\n",
- event->event, ch, ch->sess_name, ch->state);
+ pr_debug("QP event %d on ch=%p sess_name=%s-%d state=%s\n",
+ event->event, ch, ch->sess_name, ch->qp->qp_num,
+ get_ch_state_name(ch->state));
switch (event->event) {
case IB_EVENT_COMM_EST:
@@ -1985,8 +1986,8 @@ static void __srpt_close_all_ch(struct srpt_port *sport)
list_for_each_entry(nexus, &sport->nexus_list, entry) {
list_for_each_entry(ch, &nexus->ch_list, list) {
if (srpt_disconnect_ch(ch) >= 0)
- pr_info("Closing channel %s because target %s_%d has been disabled\n",
- ch->sess_name,
+ pr_info("Closing channel %s-%d because target %s_%d has been disabled\n",
+ ch->sess_name, ch->qp->qp_num,
dev_name(&sport->sdev->device->dev),
sport->port);
srpt_close_ch(ch);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] RDMA/srpt: Reduce max_recv_sge to 1
2020-05-22 21:33 [PATCH 0/4] SRP initiator and target patches for kernel v5.8 Bart Van Assche
2020-05-22 21:33 ` [PATCH 1/4] RDMA/srp: Make the channel count configurable per target Bart Van Assche
2020-05-22 21:33 ` [PATCH 2/4] RDMA/srpt: Make debug output more detailed Bart Van Assche
@ 2020-05-22 21:33 ` Bart Van Assche
2020-05-22 21:33 ` [PATCH 4/4] RDMA/srpt: Increase max_send_sge Bart Van Assche
3 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2020-05-22 21:33 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Leon Romanovsky, Doug Ledford, linux-rdma, Bart Van Assche,
Laurence Oberman, Kamal Heib
Since srpt_post_recv() always sets num_sge to 1, reduce the max_recv_sge
parameter that is used at queue pair allocation time to 1.
Cc: Laurence Oberman <loberman@redhat.com>
Cc: Kamal Heib <kamalheib1@gmail.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/infiniband/ulp/srpt/ib_srpt.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index a39ad9fc4224..1ad3cc7c553a 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1818,16 +1818,12 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
qp_init->cap.max_rdma_ctxs = sq_size / 2;
qp_init->cap.max_send_sge = min(attrs->max_send_sge,
SRPT_MAX_SG_PER_WQE);
- qp_init->cap.max_recv_sge = min(attrs->max_recv_sge,
- SRPT_MAX_SG_PER_WQE);
+ qp_init->cap.max_recv_sge = 1;
qp_init->port_num = ch->sport->port;
- if (sdev->use_srq) {
+ if (sdev->use_srq)
qp_init->srq = sdev->srq;
- } else {
+ else
qp_init->cap.max_recv_wr = ch->rq_size;
- qp_init->cap.max_recv_sge = min(attrs->max_recv_sge,
- SRPT_MAX_SG_PER_WQE);
- }
if (ch->using_rdma_cm) {
ret = rdma_create_qp(ch->rdma_cm.cm_id, sdev->pd, qp_init);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] RDMA/srpt: Increase max_send_sge
2020-05-22 21:33 [PATCH 0/4] SRP initiator and target patches for kernel v5.8 Bart Van Assche
` (2 preceding siblings ...)
2020-05-22 21:33 ` [PATCH 3/4] RDMA/srpt: Reduce max_recv_sge to 1 Bart Van Assche
@ 2020-05-22 21:33 ` Bart Van Assche
3 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2020-05-22 21:33 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Leon Romanovsky, Doug Ledford, linux-rdma, Bart Van Assche,
Laurence Oberman, Kamal Heib
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.
See also commit f95ccffc715b ("IB/mlx4: Use 4K pages for kernel QP's WQE
buffer").
Cc: Laurence Oberman <loberman@redhat.com>
Cc: Kamal Heib <kamalheib1@gmail.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/infiniband/ulp/srpt/ib_srpt.c | 25 +++++++++++++++++++++++--
drivers/infiniband/ulp/srpt/ib_srpt.h | 5 -----
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 1ad3cc7c553a..8d9b8664ea75 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1781,8 +1781,30 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
struct srpt_device *sdev = sport->sdev;
const struct ib_device_attr *attrs = &sdev->device->attrs;
int sq_size = sport->port_attrib.srp_sq_size;
+ unsigned int max_sge_delta = 0;
int i, ret;
+ switch (sdev->device->ops.driver_id) {
+ case RDMA_DRIVER_MLX4:
+ /*
+ * The smallest max_sge_delta value that works with
+ * ConnectX-3 firmware version 2.42.5000.
+ */
+ max_sge_delta = 2;
+ break;
+ case RDMA_DRIVER_MTHCA:
+ /*
+ * From the OFED release notes: In mem-free devices, RC
+ * QPs can be created with a maximum of (max_sge - 1)
+ * entries only. See also
+ * https://git.openfabrics.org/?p=compat-rdma/docs.git;a=blob;f=release_notes/mthca_release_notes.txt;h=40f3c4ea77a07fe5ded888b8417530471e89d87b.
+ */
+ max_sge_delta = 1;
+ break;
+ default:
+ break;
+ }
+
WARN_ON(ch->rq_size < 1);
ret = -ENOMEM;
@@ -1816,8 +1838,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
*/
qp_init->cap.max_send_wr = min(sq_size / 2, attrs->max_qp_wr);
qp_init->cap.max_rdma_ctxs = sq_size / 2;
- qp_init->cap.max_send_sge = min(attrs->max_send_sge,
- SRPT_MAX_SG_PER_WQE);
+ qp_init->cap.max_send_sge = attrs->max_send_sge - max_sge_delta;
qp_init->cap.max_recv_sge = 1;
qp_init->port_num = ch->sport->port;
if (sdev->use_srq)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 2e1a69840857..f31c349d07a1 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -105,11 +105,6 @@ enum {
SRP_CMD_ACA = 0x4,
SRPT_DEF_SG_TABLESIZE = 128,
- /*
- * An experimentally determined value that avoids that QP creation
- * fails due to "swiotlb buffer is full" on systems using the swiotlb.
- */
- SRPT_MAX_SG_PER_WQE = 16,
MIN_SRPT_SQ_SIZE = 16,
DEF_SRPT_SQ_SIZE = 4096,
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] RDMA/srp: Make the channel count configurable per target
2020-05-22 21:33 ` [PATCH 1/4] RDMA/srp: Make the channel count configurable per target Bart Van Assche
@ 2020-05-23 13:55 ` Leon Romanovsky
2020-05-23 15:22 ` Bart Van Assche
0 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2020-05-23 13:55 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jason Gunthorpe, Doug Ledford, linux-rdma, Laurence Oberman, Kamal Heib
On Fri, May 22, 2020 at 02:33:38PM -0700, Bart Van Assche wrote:
> Increase the flexibility of the SRP initiator driver by making the channel
> count configurable per target instead of only providing a kernel module
> parameter for configuring the channel count.
>
> Cc: Laurence Oberman <loberman@redhat.com>
> Cc: Kamal Heib <kamalheib1@gmail.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/infiniband/ulp/srp/ib_srp.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index 00b4f88b113e..d686c39710c0 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -3424,6 +3424,7 @@ enum {
> SRP_OPT_IP_DEST = 1 << 16,
> SRP_OPT_TARGET_CAN_QUEUE= 1 << 17,
> SRP_OPT_MAX_IT_IU_SIZE = 1 << 18,
> + SRP_OPT_CH_COUNT = 1 << 19,
> };
>
> static unsigned int srp_opt_mandatory[] = {
> @@ -3457,6 +3458,7 @@ static const match_table_t srp_opt_tokens = {
> { SRP_OPT_IP_SRC, "src=%s" },
> { SRP_OPT_IP_DEST, "dest=%s" },
> { SRP_OPT_MAX_IT_IU_SIZE, "max_it_iu_size=%d" },
> + { SRP_OPT_CH_COUNT, "ch_count=%d", },
Why did you use %d and not %u?
Thanks
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] RDMA/srp: Make the channel count configurable per target
2020-05-23 13:55 ` Leon Romanovsky
@ 2020-05-23 15:22 ` Bart Van Assche
2020-05-24 6:20 ` Leon Romanovsky
0 siblings, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2020-05-23 15:22 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Jason Gunthorpe, Doug Ledford, linux-rdma, Laurence Oberman, Kamal Heib
On 2020-05-23 06:55, Leon Romanovsky wrote:
> On Fri, May 22, 2020 at 02:33:38PM -0700, Bart Van Assche wrote:
>> Increase the flexibility of the SRP initiator driver by making the channel
>> count configurable per target instead of only providing a kernel module
>> parameter for configuring the channel count.
>>
>> Cc: Laurence Oberman <loberman@redhat.com>
>> Cc: Kamal Heib <kamalheib1@gmail.com>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>> drivers/infiniband/ulp/srp/ib_srp.c | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
>> index 00b4f88b113e..d686c39710c0 100644
>> --- a/drivers/infiniband/ulp/srp/ib_srp.c
>> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
>> @@ -3424,6 +3424,7 @@ enum {
>> SRP_OPT_IP_DEST = 1 << 16,
>> SRP_OPT_TARGET_CAN_QUEUE= 1 << 17,
>> SRP_OPT_MAX_IT_IU_SIZE = 1 << 18,
>> + SRP_OPT_CH_COUNT = 1 << 19,
>> };
>>
>> static unsigned int srp_opt_mandatory[] = {
>> @@ -3457,6 +3458,7 @@ static const match_table_t srp_opt_tokens = {
>> { SRP_OPT_IP_SRC, "src=%s" },
>> { SRP_OPT_IP_DEST, "dest=%s" },
>> { SRP_OPT_MAX_IT_IU_SIZE, "max_it_iu_size=%d" },
>> + { SRP_OPT_CH_COUNT, "ch_count=%d", },
>
> Why did you use %d and not %u?
Hi Leon,
There is more kernel code that uses %d for unsigned integers. Anyway, if
someone cares enough I can change %d into %u.
Bart.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] RDMA/srp: Make the channel count configurable per target
2020-05-23 15:22 ` Bart Van Assche
@ 2020-05-24 6:20 ` Leon Romanovsky
0 siblings, 0 replies; 8+ messages in thread
From: Leon Romanovsky @ 2020-05-24 6:20 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jason Gunthorpe, Doug Ledford, linux-rdma, Laurence Oberman, Kamal Heib
On Sat, May 23, 2020 at 08:22:19AM -0700, Bart Van Assche wrote:
> On 2020-05-23 06:55, Leon Romanovsky wrote:
> > On Fri, May 22, 2020 at 02:33:38PM -0700, Bart Van Assche wrote:
> >> Increase the flexibility of the SRP initiator driver by making the channel
> >> count configurable per target instead of only providing a kernel module
> >> parameter for configuring the channel count.
> >>
> >> Cc: Laurence Oberman <loberman@redhat.com>
> >> Cc: Kamal Heib <kamalheib1@gmail.com>
> >> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> >> ---
> >> drivers/infiniband/ulp/srp/ib_srp.c | 21 ++++++++++++++++-----
> >> 1 file changed, 16 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> >> index 00b4f88b113e..d686c39710c0 100644
> >> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> >> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> >> @@ -3424,6 +3424,7 @@ enum {
> >> SRP_OPT_IP_DEST = 1 << 16,
> >> SRP_OPT_TARGET_CAN_QUEUE= 1 << 17,
> >> SRP_OPT_MAX_IT_IU_SIZE = 1 << 18,
> >> + SRP_OPT_CH_COUNT = 1 << 19,
> >> };
> >>
> >> static unsigned int srp_opt_mandatory[] = {
> >> @@ -3457,6 +3458,7 @@ static const match_table_t srp_opt_tokens = {
> >> { SRP_OPT_IP_SRC, "src=%s" },
> >> { SRP_OPT_IP_DEST, "dest=%s" },
> >> { SRP_OPT_MAX_IT_IU_SIZE, "max_it_iu_size=%d" },
> >> + { SRP_OPT_CH_COUNT, "ch_count=%d", },
> >
> > Why did you use %d and not %u?
>
> Hi Leon,
>
> There is more kernel code that uses %d for unsigned integers. Anyway, if
> someone cares enough I can change %d into %u.
I don't have strong opinion about that.
Thanks
>
> Bart.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-24 6:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22 21:33 [PATCH 0/4] SRP initiator and target patches for kernel v5.8 Bart Van Assche
2020-05-22 21:33 ` [PATCH 1/4] RDMA/srp: Make the channel count configurable per target Bart Van Assche
2020-05-23 13:55 ` Leon Romanovsky
2020-05-23 15:22 ` Bart Van Assche
2020-05-24 6:20 ` Leon Romanovsky
2020-05-22 21:33 ` [PATCH 2/4] RDMA/srpt: Make debug output more detailed Bart Van Assche
2020-05-22 21:33 ` [PATCH 3/4] RDMA/srpt: Reduce max_recv_sge to 1 Bart Van Assche
2020-05-22 21:33 ` [PATCH 4/4] RDMA/srpt: Increase max_send_sge Bart Van Assche
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.