* [PATCHv2 for-next 01/19] RDMA/rtrs-srv: Kill reject_w_econnreset label
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 02/19] RDMA/rtrs-clt: Remove MAX_SESS_QUEUE_DEPTH from rtrs_send_sess_info Gioh Kim
` (17 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Guoqing Jiang, Guoqing Jiang, Gioh Kim
From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
We can goto reject_w_err label after initialize err with -ECONNRESET.
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 740cafffb7ef..3d09d01e34b4 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1780,33 +1780,33 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
u16 version, con_num, cid;
u16 recon_cnt;
- int err;
+ int err = -ECONNRESET;
if (len < sizeof(*msg)) {
pr_err("Invalid RTRS connection request\n");
- goto reject_w_econnreset;
+ goto reject_w_err;
}
if (le16_to_cpu(msg->magic) != RTRS_MAGIC) {
pr_err("Invalid RTRS magic\n");
- goto reject_w_econnreset;
+ goto reject_w_err;
}
version = le16_to_cpu(msg->version);
if (version >> 8 != RTRS_PROTO_VER_MAJOR) {
pr_err("Unsupported major RTRS version: %d, expected %d\n",
version >> 8, RTRS_PROTO_VER_MAJOR);
- goto reject_w_econnreset;
+ goto reject_w_err;
}
con_num = le16_to_cpu(msg->cid_num);
if (con_num > 4096) {
/* Sanity check */
pr_err("Too many connections requested: %d\n", con_num);
- goto reject_w_econnreset;
+ goto reject_w_err;
}
cid = le16_to_cpu(msg->cid);
if (cid >= con_num) {
/* Sanity check */
pr_err("Incorrect cid: %d >= %d\n", cid, con_num);
- goto reject_w_econnreset;
+ goto reject_w_err;
}
recon_cnt = le16_to_cpu(msg->recon_cnt);
srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
@@ -1826,7 +1826,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
rtrs_err(s, "Session in wrong state: %s\n",
rtrs_srv_state_str(sess->state));
mutex_unlock(&srv->paths_mutex);
- goto reject_w_econnreset;
+ goto reject_w_err;
}
/*
* Sanity checks
@@ -1835,13 +1835,13 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
rtrs_err(s, "Incorrect request: %d, %d\n",
cid, con_num);
mutex_unlock(&srv->paths_mutex);
- goto reject_w_econnreset;
+ goto reject_w_err;
}
if (s->con[cid]) {
rtrs_err(s, "Connection already exists: %d\n",
cid);
mutex_unlock(&srv->paths_mutex);
- goto reject_w_econnreset;
+ goto reject_w_err;
}
} else {
sess = __alloc_sess(srv, cm_id, con_num, recon_cnt,
@@ -1882,9 +1882,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
reject_w_err:
return rtrs_rdma_do_reject(cm_id, err);
-reject_w_econnreset:
- return rtrs_rdma_do_reject(cm_id, -ECONNRESET);
-
close_and_return_err:
mutex_unlock(&srv->paths_mutex);
close_sess(sess);
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 02/19] RDMA/rtrs-clt: Remove MAX_SESS_QUEUE_DEPTH from rtrs_send_sess_info
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 01/19] RDMA/rtrs-srv: Kill reject_w_econnreset label Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 03/19] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection Gioh Kim
` (16 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Gioh Kim, Gioh Kim
From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
Client receives queue_depth value from server. There is no need
to use MAX_SESS_QUEUE_DEPTH value.
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 5e94686bb22d..930a1b496f84 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2455,7 +2455,7 @@ static int rtrs_send_sess_info(struct rtrs_clt_sess *sess)
int err;
rx_sz = sizeof(struct rtrs_msg_info_rsp);
- rx_sz += sizeof(u64) * MAX_SESS_QUEUE_DEPTH;
+ rx_sz += sizeof(struct rtrs_sg_desc) * sess->queue_depth;
tx_iu = rtrs_iu_alloc(1, sizeof(struct rtrs_msg_info_req), GFP_KERNEL,
sess->s.dev->ib_dev, DMA_TO_DEVICE,
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 03/19] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 01/19] RDMA/rtrs-srv: Kill reject_w_econnreset label Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 02/19] RDMA/rtrs-clt: Remove MAX_SESS_QUEUE_DEPTH from rtrs_send_sess_info Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-25 20:18 ` Jason Gunthorpe
2021-05-17 9:18 ` [PATCHv2 for-next 04/19] RDMA/rtrs-srv: Clean up the code in __rtrs_srv_change_state Gioh Kim
` (15 subsequent siblings)
18 siblings, 1 reply; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Md Haris Iqbal, Gioh Kim
From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
It was difficult to find out why it failed to establish RDMA
connection. This patch adds some messages to show which function
has failed why.
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 3d09d01e34b4..df17dd4c1e28 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
* If this request is not the first connection request from the
* client for this session then fail and return error.
*/
- if (!first_conn)
+ if (!first_conn) {
+ pr_err("Error: Not the first connection request for this session\n");
return ERR_PTR(-ENXIO);
+ }
/* need to allocate a new srv */
srv = kzalloc(sizeof(*srv), GFP_KERNEL);
@@ -1812,6 +1814,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
if (IS_ERR(srv)) {
err = PTR_ERR(srv);
+ pr_err("get_or_create_srv(), error %d\n", err);
goto reject_w_err;
}
mutex_lock(&srv->paths_mutex);
@@ -1850,11 +1853,13 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
mutex_unlock(&srv->paths_mutex);
put_srv(srv);
err = PTR_ERR(sess);
+ pr_err("RTRS server session allocation failed: %d\n", err);
goto reject_w_err;
}
}
err = create_con(sess, cm_id, cid);
if (err) {
+ rtrs_err((&sess->s), "create_con(), error %d\n", err);
(void)rtrs_rdma_do_reject(cm_id, err);
/*
* Since session has other connections we follow normal way
@@ -1865,6 +1870,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
}
err = rtrs_rdma_do_accept(sess, cm_id);
if (err) {
+ rtrs_err((&sess->s), "rtrs_rdma_do_accept(), error %d\n", err);
(void)rtrs_rdma_do_reject(cm_id, err);
/*
* Since current connection was successfully added to the
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCHv2 for-next 03/19] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection
2021-05-17 9:18 ` [PATCHv2 for-next 03/19] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection Gioh Kim
@ 2021-05-25 20:18 ` Jason Gunthorpe
2021-05-26 9:28 ` Haris Iqbal
0 siblings, 1 reply; 28+ messages in thread
From: Jason Gunthorpe @ 2021-05-25 20:18 UTC (permalink / raw)
To: Gioh Kim
Cc: linux-rdma, bvanassche, leon, dledford, haris.iqbal, jinpu.wang,
Md Haris Iqbal
On Mon, May 17, 2021 at 11:18:27AM +0200, Gioh Kim wrote:
> From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
>
> It was difficult to find out why it failed to establish RDMA
> connection. This patch adds some messages to show which function
> has failed why.
>
> Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index 3d09d01e34b4..df17dd4c1e28 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
> * If this request is not the first connection request from the
> * client for this session then fail and return error.
> */
> - if (!first_conn)
> + if (!first_conn) {
> + pr_err("Error: Not the first connection request for this session\n");
> return ERR_PTR(-ENXIO);
You really shouldn't be printing based on attacker controlled data..
Jason
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 for-next 03/19] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection
2021-05-25 20:18 ` Jason Gunthorpe
@ 2021-05-26 9:28 ` Haris Iqbal
2021-05-26 16:07 ` Jason Gunthorpe
0 siblings, 1 reply; 28+ messages in thread
From: Haris Iqbal @ 2021-05-26 9:28 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Gioh Kim, linux-rdma, bvanassche, Leon Romanovsky, Doug Ledford,
Jinpu Wang, Md Haris Iqbal
On Tue, May 25, 2021 at 10:18 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Mon, May 17, 2021 at 11:18:27AM +0200, Gioh Kim wrote:
> > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> >
> > It was difficult to find out why it failed to establish RDMA
> > connection. This patch adds some messages to show which function
> > has failed why.
> >
> > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > ---
> > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > index 3d09d01e34b4..df17dd4c1e28 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > @@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
> > * If this request is not the first connection request from the
> > * client for this session then fail and return error.
> > */
> > - if (!first_conn)
> > + if (!first_conn) {
> > + pr_err("Error: Not the first connection request for this session\n");
> > return ERR_PTR(-ENXIO);
>
> You really shouldn't be printing based on attacker controlled data..
I want to make sure I understand correctly. Did you mean that an
attacker can bombard a server with such connection request, which can
lead to uncontrolled prints, and possibly DOS?
If so, would a ratelimited print be better?
>
> Jason
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 for-next 03/19] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection
2021-05-26 9:28 ` Haris Iqbal
@ 2021-05-26 16:07 ` Jason Gunthorpe
2021-05-27 9:26 ` Haris Iqbal
0 siblings, 1 reply; 28+ messages in thread
From: Jason Gunthorpe @ 2021-05-26 16:07 UTC (permalink / raw)
To: Haris Iqbal
Cc: Gioh Kim, linux-rdma, bvanassche, Leon Romanovsky, Doug Ledford,
Jinpu Wang, Md Haris Iqbal
On Wed, May 26, 2021 at 11:28:41AM +0200, Haris Iqbal wrote:
> On Tue, May 25, 2021 at 10:18 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Mon, May 17, 2021 at 11:18:27AM +0200, Gioh Kim wrote:
> > > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> > >
> > > It was difficult to find out why it failed to establish RDMA
> > > connection. This patch adds some messages to show which function
> > > has failed why.
> > >
> > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > index 3d09d01e34b4..df17dd4c1e28 100644
> > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > @@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
> > > * If this request is not the first connection request from the
> > > * client for this session then fail and return error.
> > > */
> > > - if (!first_conn)
> > > + if (!first_conn) {
> > > + pr_err("Error: Not the first connection request for this session\n");
> > > return ERR_PTR(-ENXIO);
> >
> > You really shouldn't be printing based on attacker controlled data..
>
> I want to make sure I understand correctly. Did you mean that an
> attacker can bombard a server with such connection request, which can
> lead to uncontrolled prints, and possibly DOS?
Yes
> If so, would a ratelimited print be better?
Probably
Jason
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 for-next 03/19] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection
2021-05-26 16:07 ` Jason Gunthorpe
@ 2021-05-27 9:26 ` Haris Iqbal
0 siblings, 0 replies; 28+ messages in thread
From: Haris Iqbal @ 2021-05-27 9:26 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Gioh Kim, RDMA mailing list, Bart Van Assche, Leon Romanovsky,
Doug Ledford, Jinpu Wang, Md Haris Iqbal
On Wed, May 26, 2021 at 6:07 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Wed, May 26, 2021 at 11:28:41AM +0200, Haris Iqbal wrote:
> > On Tue, May 25, 2021 at 10:18 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> > >
> > > On Mon, May 17, 2021 at 11:18:27AM +0200, Gioh Kim wrote:
> > > > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> > > >
> > > > It was difficult to find out why it failed to establish RDMA
> > > > connection. This patch adds some messages to show which function
> > > > has failed why.
> > > >
> > > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > > > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > > > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
> > > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > > index 3d09d01e34b4..df17dd4c1e28 100644
> > > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > > @@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
> > > > * If this request is not the first connection request from the
> > > > * client for this session then fail and return error.
> > > > */
> > > > - if (!first_conn)
> > > > + if (!first_conn) {
> > > > + pr_err("Error: Not the first connection request for this session\n");
> > > > return ERR_PTR(-ENXIO);
> > >
> > > You really shouldn't be printing based on attacker controlled data..
> >
> > I want to make sure I understand correctly. Did you mean that an
> > attacker can bombard a server with such connection request, which can
> > lead to uncontrolled prints, and possibly DOS?
>
> Yes
>
> > If so, would a ratelimited print be better?
>
> Probably
Alright.. Will change this print to a ratelimited one, and send the
updates patches.
Thanks.
>
> Jason
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCHv2 for-next 04/19] RDMA/rtrs-srv: Clean up the code in __rtrs_srv_change_state
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (2 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 03/19] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 05/19] RDMA/rtrs: Change MAX_SESS_QUEUE_DEPTH Gioh Kim
` (14 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Guoqing Jiang, Guoqing Jiang, Gioh Kim
From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
No need to use double switch to check the change of state everywhere,
let's change them to "if" to reduce size.
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index df17dd4c1e28..74a5dfe85813 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -77,32 +77,17 @@ static bool __rtrs_srv_change_state(struct rtrs_srv_sess *sess,
old_state = sess->state;
switch (new_state) {
case RTRS_SRV_CONNECTED:
- switch (old_state) {
- case RTRS_SRV_CONNECTING:
+ if (old_state == RTRS_SRV_CONNECTING)
changed = true;
- fallthrough;
- default:
- break;
- }
break;
case RTRS_SRV_CLOSING:
- switch (old_state) {
- case RTRS_SRV_CONNECTING:
- case RTRS_SRV_CONNECTED:
+ if (old_state == RTRS_SRV_CONNECTING ||
+ old_state == RTRS_SRV_CONNECTED)
changed = true;
- fallthrough;
- default:
- break;
- }
break;
case RTRS_SRV_CLOSED:
- switch (old_state) {
- case RTRS_SRV_CLOSING:
+ if (old_state == RTRS_SRV_CLOSING)
changed = true;
- fallthrough;
- default:
- break;
- }
break;
default:
break;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 05/19] RDMA/rtrs: Change MAX_SESS_QUEUE_DEPTH
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (3 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 04/19] RDMA/rtrs-srv: Clean up the code in __rtrs_srv_change_state Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 06/19] RDMA/rtrs: Define MIN_CHUNK_SIZE Gioh Kim
` (13 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Gioh Kim, Gioh Kim
From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
Max IB immediate data size is 2^28 (MAX_IMM_PAYL_BITS)
and the minimum chunk size is 4096 (2^12).
Therefore the maximum sess_queue_depth is 65536 (2^16).
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-pri.h | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
index bcad5e2168c5..62924ad5362d 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
@@ -47,12 +47,15 @@ enum {
MAX_PATHS_NUM = 128,
/*
- * With the size of struct rtrs_permit allocated on the client, 4K
- * is the maximum number of rtrs_permits we can allocate. This number is
- * also used on the client to allocate the IU for the user connection
- * to receive the RDMA addresses from the server.
+ * Max IB immediate data size is 2^28 (MAX_IMM_PAYL_BITS)
+ * and the minimum chunk size is 4096 (2^12).
+ * So the maximum sess_queue_depth is 65536 (2^16) in theory.
+ * But mempool_create, create_qp and ib_post_send fail with
+ * "cannot allocate memory" error if sess_queue_depth is too big.
+ * Therefore the pratical max value of sess_queue_depth is
+ * somewhere between 1 and 65536 and it depends on the system.
*/
- MAX_SESS_QUEUE_DEPTH = 4096,
+ MAX_SESS_QUEUE_DEPTH = 65536,
RTRS_HB_INTERVAL_MS = 5000,
RTRS_HB_MISSED_MAX = 5,
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 06/19] RDMA/rtrs: Define MIN_CHUNK_SIZE
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (4 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 05/19] RDMA/rtrs: Change MAX_SESS_QUEUE_DEPTH Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 07/19] RDMA/rtrs: Use strscpy instead of strlcpy Gioh Kim
` (12 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Gioh Kim, Gioh Kim
From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
Define MIN_CHUNK_SIZE to replace the hard-coding number.
We need 4k for metadata, so MIN_CHUNK_SIZE should be at least 8k.
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-pri.h | 1 +
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
index 62924ad5362d..a092f7d7c7b3 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
@@ -56,6 +56,7 @@ enum {
* somewhere between 1 and 65536 and it depends on the system.
*/
MAX_SESS_QUEUE_DEPTH = 65536,
+ MIN_CHUNK_SIZE = 8192,
RTRS_HB_INTERVAL_MS = 5000,
RTRS_HB_MISSED_MAX = 5,
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 74a5dfe85813..02bc704667a8 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -2165,9 +2165,9 @@ static int check_module_params(void)
sess_queue_depth, 1, MAX_SESS_QUEUE_DEPTH);
return -EINVAL;
}
- if (max_chunk_size < 4096 || !is_power_of_2(max_chunk_size)) {
+ if (max_chunk_size < MIN_CHUNK_SIZE || !is_power_of_2(max_chunk_size)) {
pr_err("Invalid max_chunk_size value %d, has to be >= %d and should be power of two.\n",
- max_chunk_size, 4096);
+ max_chunk_size, MIN_CHUNK_SIZE);
return -EINVAL;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 07/19] RDMA/rtrs: Use strscpy instead of strlcpy
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (5 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 06/19] RDMA/rtrs: Define MIN_CHUNK_SIZE Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 08/19] RDMA/rtrs-clt: Kill rtrs_clt_{start,stop}_hb Gioh Kim
` (11 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Dima Stepanov, Dima Stepanov, Gioh Kim
From: Dima Stepanov <dmitrii.stepanov@cloud.ionos.com>
During checkpatch analyzing the following warning message was found:
WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
Fix it by using strscpy calls instead of strlcpy.
Signed-off-by: Dima Stepanov <dmitrii.stepanov@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 4 ++--
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 930a1b496f84..a2505ea4a7a2 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1499,7 +1499,7 @@ static struct rtrs_clt_sess *alloc_sess(struct rtrs_clt *clt,
if (path->src)
memcpy(&sess->s.src_addr, path->src,
rdma_addr_size((struct sockaddr *)path->src));
- strlcpy(sess->s.sessname, clt->sessname, sizeof(sess->s.sessname));
+ strscpy(sess->s.sessname, clt->sessname, sizeof(sess->s.sessname));
sess->s.con_num = con_num;
sess->clt = clt;
sess->max_pages_per_mr = max_segments * max_segment_size >> 12;
@@ -2644,7 +2644,7 @@ static struct rtrs_clt *alloc_clt(const char *sessname, size_t paths_num,
clt->priv = priv;
clt->link_ev = link_ev;
clt->mp_policy = MP_POLICY_MIN_INFLIGHT;
- strlcpy(clt->sessname, sessname, sizeof(clt->sessname));
+ strscpy(clt->sessname, sessname, sizeof(clt->sessname));
init_waitqueue_head(&clt->permits_wait);
mutex_init(&clt->paths_ev_mutex);
mutex_init(&clt->paths_mutex);
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 02bc704667a8..afa63f06586b 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -780,7 +780,7 @@ static int process_info_req(struct rtrs_srv_con *con,
rwr = kcalloc(sess->mrs_num, sizeof(*rwr), GFP_KERNEL);
if (unlikely(!rwr))
return -ENOMEM;
- strlcpy(sess->s.sessname, msg->sessname, sizeof(sess->s.sessname));
+ strscpy(sess->s.sessname, msg->sessname, sizeof(sess->s.sessname));
tx_sz = sizeof(*rsp);
tx_sz += sizeof(rsp->desc[0]) * sess->mrs_num;
@@ -1261,7 +1261,7 @@ int rtrs_srv_get_sess_name(struct rtrs_srv *srv, char *sessname, size_t len)
list_for_each_entry(sess, &srv->paths_list, s.entry) {
if (sess->state != RTRS_SRV_CONNECTED)
continue;
- strlcpy(sessname, sess->s.sessname,
+ strscpy(sessname, sess->s.sessname,
min_t(size_t, sizeof(sess->s.sessname), len));
err = 0;
break;
@@ -1715,7 +1715,7 @@ static struct rtrs_srv_sess *__alloc_sess(struct rtrs_srv *srv,
path.src = &sess->s.src_addr;
path.dst = &sess->s.dst_addr;
rtrs_addr_to_str(&path, str, sizeof(str));
- strlcpy(sess->s.sessname, str, sizeof(sess->s.sessname));
+ strscpy(sess->s.sessname, str, sizeof(sess->s.sessname));
sess->s.con_num = con_num;
sess->s.recon_cnt = recon_cnt;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 08/19] RDMA/rtrs-clt: Kill rtrs_clt_{start,stop}_hb
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (6 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 07/19] RDMA/rtrs: Use strscpy instead of strlcpy Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 09/19] RDMA/rtrs-clt: Kill rtrs_clt_disconnect_from_sysfs Gioh Kim
` (10 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Guoqing Jiang, Guoqing Jiang, Gioh Kim
From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
The two wrappers are not needed since we can call rtrs_{start,stop}_hb
directly.
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index a2505ea4a7a2..0f19c133b7cc 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1449,16 +1449,6 @@ static void rtrs_clt_init_hb(struct rtrs_clt_sess *sess)
rtrs_wq);
}
-static void rtrs_clt_start_hb(struct rtrs_clt_sess *sess)
-{
- rtrs_start_hb(&sess->s);
-}
-
-static void rtrs_clt_stop_hb(struct rtrs_clt_sess *sess)
-{
- rtrs_stop_hb(&sess->s);
-}
-
static void rtrs_clt_reconnect_work(struct work_struct *work);
static void rtrs_clt_close_work(struct work_struct *work);
@@ -2088,7 +2078,7 @@ static void rtrs_clt_stop_and_destroy_conns(struct rtrs_clt_sess *sess)
*/
synchronize_rcu();
- rtrs_clt_stop_hb(sess);
+ rtrs_stop_hb(&sess->s);
/*
* The order it utterly crucial: firstly disconnect and complete all
@@ -2281,7 +2271,7 @@ static int init_conns(struct rtrs_clt_sess *sess)
if (err)
goto destroy;
- rtrs_clt_start_hb(sess);
+ rtrs_start_hb(&sess->s);
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 09/19] RDMA/rtrs-clt: Kill rtrs_clt_disconnect_from_sysfs
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (7 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 08/19] RDMA/rtrs-clt: Kill rtrs_clt_{start,stop}_hb Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 10/19] RDMA/rtrs-srv: Kill __rtrs_srv_change_state Gioh Kim
` (9 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Guoqing Jiang, Guoqing Jiang, Gioh Kim
From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
The function is just a wrapper of rtrs_clt_close_conns, let's call
rtrs_clt_close_conns directly.
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c | 5 +----
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 9 +--------
drivers/infiniband/ulp/rtrs/rtrs-clt.h | 2 +-
3 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
index 7d53d18a5004..4ee592ccf979 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
@@ -250,7 +250,6 @@ static ssize_t rtrs_clt_disconnect_store(struct kobject *kobj,
const char *buf, size_t count)
{
struct rtrs_clt_sess *sess;
- int ret;
sess = container_of(kobj, struct rtrs_clt_sess, kobj);
if (!sysfs_streq(buf, "1")) {
@@ -258,9 +257,7 @@ static ssize_t rtrs_clt_disconnect_store(struct kobject *kobj,
attr->attr.name, buf);
return -EINVAL;
}
- ret = rtrs_clt_disconnect_from_sysfs(sess);
- if (ret)
- return ret;
+ rtrs_clt_close_conns(sess, true);
return count;
}
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 0f19c133b7cc..1de72784e1c0 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1849,7 +1849,7 @@ static int rtrs_rdma_conn_rejected(struct rtrs_clt_con *con,
return -ECONNRESET;
}
-static void rtrs_clt_close_conns(struct rtrs_clt_sess *sess, bool wait)
+void rtrs_clt_close_conns(struct rtrs_clt_sess *sess, bool wait)
{
if (rtrs_clt_change_state_get_old(sess, RTRS_CLT_CLOSING, NULL))
queue_work(rtrs_wq, &sess->close_work);
@@ -2824,13 +2824,6 @@ int rtrs_clt_reconnect_from_sysfs(struct rtrs_clt_sess *sess)
return err;
}
-int rtrs_clt_disconnect_from_sysfs(struct rtrs_clt_sess *sess)
-{
- rtrs_clt_close_conns(sess, true);
-
- return 0;
-}
-
int rtrs_clt_remove_path_from_sysfs(struct rtrs_clt_sess *sess,
const struct attribute *sysfs_self)
{
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.h b/drivers/infiniband/ulp/rtrs/rtrs-clt.h
index ef15927dfeda..0a0ea43d2b46 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.h
@@ -203,7 +203,7 @@ static inline struct rtrs_permit *get_permit(struct rtrs_clt *clt, int idx)
}
int rtrs_clt_reconnect_from_sysfs(struct rtrs_clt_sess *sess);
-int rtrs_clt_disconnect_from_sysfs(struct rtrs_clt_sess *sess);
+void rtrs_clt_close_conns(struct rtrs_clt_sess *sess, bool wait);
int rtrs_clt_create_path_from_sysfs(struct rtrs_clt *clt,
struct rtrs_addr *addr);
int rtrs_clt_remove_path_from_sysfs(struct rtrs_clt_sess *sess,
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 10/19] RDMA/rtrs-srv: Kill __rtrs_srv_change_state
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (8 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 09/19] RDMA/rtrs-clt: Kill rtrs_clt_disconnect_from_sysfs Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 11/19] RDMA/rtrs-clt: Remove redundant 'break' Gioh Kim
` (8 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Guoqing Jiang, Guoqing Jiang, Gioh Kim
From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
No need since the only user is rtrs_srv_change_state.
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index afa63f06586b..54a29285240d 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -67,13 +67,13 @@ static inline struct rtrs_srv_sess *to_srv_sess(struct rtrs_sess *s)
return container_of(s, struct rtrs_srv_sess, s);
}
-static bool __rtrs_srv_change_state(struct rtrs_srv_sess *sess,
- enum rtrs_srv_state new_state)
+static bool rtrs_srv_change_state(struct rtrs_srv_sess *sess,
+ enum rtrs_srv_state new_state)
{
enum rtrs_srv_state old_state;
bool changed = false;
- lockdep_assert_held(&sess->state_lock);
+ spin_lock_irq(&sess->state_lock);
old_state = sess->state;
switch (new_state) {
case RTRS_SRV_CONNECTED:
@@ -94,17 +94,6 @@ static bool __rtrs_srv_change_state(struct rtrs_srv_sess *sess,
}
if (changed)
sess->state = new_state;
-
- return changed;
-}
-
-static bool rtrs_srv_change_state(struct rtrs_srv_sess *sess,
- enum rtrs_srv_state new_state)
-{
- bool changed;
-
- spin_lock_irq(&sess->state_lock);
- changed = __rtrs_srv_change_state(sess, new_state);
spin_unlock_irq(&sess->state_lock);
return changed;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 11/19] RDMA/rtrs-clt: Remove redundant 'break'
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (9 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 10/19] RDMA/rtrs-srv: Kill __rtrs_srv_change_state Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 12/19] RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats Gioh Kim
` (7 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Guoqing Jiang, Guoqing Jiang, Gioh Kim
From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
It is duplicated with the very next line
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 1de72784e1c0..757a4394fdb6 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -655,7 +655,6 @@ static void rtrs_clt_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
rtrs_err(con->c.sess, "rtrs_post_recv_empty(): %d\n",
err);
rtrs_rdma_error_recovery(con);
- break;
}
break;
case IB_WC_RECV:
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 12/19] RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (10 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 11/19] RDMA/rtrs-clt: Remove redundant 'break' Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 13/19] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight Gioh Kim
` (6 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Md Haris Iqbal, Gioh Kim
From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
When get_next_path_min_inflight is called to select the next path, it
iterates over the list of available rtrs_clt_sess (paths). It then reads
the number of inflight IOs for that path to select one which has the least
inflight IO.
But it may so happen that rtrs_clt_sess (path) is no longer in the
connected state because closing or error recovery paths can change
the status of the rtrs_clt_Sess.
For example, the client sent the heart-beat and did not get the
response, it would change the session status and stop IO processing.
The added checking of this patch can prevent accessing the broken path
and generating duplicated error messages.
It is ok if the status is changed after checking the status because
the error recovery path does not free memory and only tries to
reconnection. And also it is ok if the session is closed after checking
the status because closing the session changes the session status and
flush all IO beforing free memory. If the session is being accessed for
IO processing, the closing session will wait.
Fixes: 6a98d71daea18 ("RDMA/rtrs: client: main functionality")
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Reviewed-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 757a4394fdb6..a3d20a5c5112 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -813,6 +813,9 @@ static struct rtrs_clt_sess *get_next_path_min_inflight(struct path_it *it)
int inflight;
list_for_each_entry_rcu(sess, &clt->paths_list, s.entry) {
+ if (unlikely(READ_ONCE(sess->state) != RTRS_CLT_CONNECTED))
+ continue;
+
if (unlikely(!list_empty(raw_cpu_ptr(sess->mp_skip_entry))))
continue;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 13/19] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (11 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 12/19] RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-25 20:15 ` Jason Gunthorpe
2021-05-17 9:18 ` [PATCHv2 for-next 14/19] RDMA/rtrs: Do not reset hb_missed_max after re-connection Gioh Kim
` (5 subsequent siblings)
18 siblings, 1 reply; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Md Haris Iqbal, Gioh Kim
From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
ids_inflight is used to track the inflight IOs. But the use of atomic_t
variable can cause performance drops and can also become a performance
bottleneck.
This commit replaces the use of atomic_t with a percpu_ref structure. The
advantage it offers is, it doesn't check if the reference has fallen to 0,
until the user explicitly signals it to; and that is done by the
percpu_ref_kill() function call. After that, the percpu_ref structure
behaves like an atomic_t and for every put call, checks whether the
reference has fallen to 0 or not.
rtrs_srv_stats_rdma_to_str shows the count of ids_inflight as 0
for user-mode tools not to be confused.
Fixes: 9cb837480424e ("RDMA/rtrs: server: main functionality")
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c | 12 +++---
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 43 +++++++++++++-------
drivers/infiniband/ulp/rtrs/rtrs-srv.h | 4 +-
3 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
index e102b1368d0c..df1d7d6b1884 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
@@ -27,12 +27,10 @@ ssize_t rtrs_srv_stats_rdma_to_str(struct rtrs_srv_stats *stats,
char *page, size_t len)
{
struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
- struct rtrs_srv_sess *sess = stats->sess;
- return scnprintf(page, len, "%lld %lld %lld %lld %u\n",
- (s64)atomic64_read(&r->dir[READ].cnt),
- (s64)atomic64_read(&r->dir[READ].size_total),
- (s64)atomic64_read(&r->dir[WRITE].cnt),
- (s64)atomic64_read(&r->dir[WRITE].size_total),
- atomic_read(&sess->ids_inflight));
+ return sysfs_emit(page, "%lld %lld %lld %lldn\n",
+ (s64)atomic64_read(&r->dir[READ].cnt),
+ (s64)atomic64_read(&r->dir[READ].size_total),
+ (s64)atomic64_read(&r->dir[WRITE].cnt),
+ (s64)atomic64_read(&r->dir[WRITE].size_total));
}
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 54a29285240d..f66f2be9f519 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -111,7 +111,6 @@ static void rtrs_srv_free_ops_ids(struct rtrs_srv_sess *sess)
struct rtrs_srv *srv = sess->srv;
int i;
- WARN_ON(atomic_read(&sess->ids_inflight));
if (sess->ops_ids) {
for (i = 0; i < srv->queue_depth; i++)
free_id(sess->ops_ids[i]);
@@ -126,11 +125,19 @@ static struct ib_cqe io_comp_cqe = {
.done = rtrs_srv_rdma_done
};
+static inline void rtrs_srv_inflight_ref_release(struct percpu_ref *ref)
+{
+ struct rtrs_srv_sess *sess = container_of(ref, struct rtrs_srv_sess, ids_inflight_ref);
+
+ percpu_ref_exit(&sess->ids_inflight_ref);
+ complete(&sess->complete_done);
+}
+
static int rtrs_srv_alloc_ops_ids(struct rtrs_srv_sess *sess)
{
struct rtrs_srv *srv = sess->srv;
struct rtrs_srv_op *id;
- int i;
+ int i, ret;
sess->ops_ids = kcalloc(srv->queue_depth, sizeof(*sess->ops_ids),
GFP_KERNEL);
@@ -144,8 +151,14 @@ static int rtrs_srv_alloc_ops_ids(struct rtrs_srv_sess *sess)
sess->ops_ids[i] = id;
}
- init_waitqueue_head(&sess->ids_waitq);
- atomic_set(&sess->ids_inflight, 0);
+
+ ret = percpu_ref_init(&sess->ids_inflight_ref,
+ rtrs_srv_inflight_ref_release, 0, GFP_KERNEL);
+ if (ret) {
+ pr_err("Percpu reference init failed\n");
+ goto err;
+ }
+ init_completion(&sess->complete_done);
return 0;
@@ -156,21 +169,14 @@ static int rtrs_srv_alloc_ops_ids(struct rtrs_srv_sess *sess)
static inline void rtrs_srv_get_ops_ids(struct rtrs_srv_sess *sess)
{
- atomic_inc(&sess->ids_inflight);
+ percpu_ref_get(&sess->ids_inflight_ref);
}
static inline void rtrs_srv_put_ops_ids(struct rtrs_srv_sess *sess)
{
- if (atomic_dec_and_test(&sess->ids_inflight))
- wake_up(&sess->ids_waitq);
+ percpu_ref_put(&sess->ids_inflight_ref);
}
-static void rtrs_srv_wait_ops_ids(struct rtrs_srv_sess *sess)
-{
- wait_event(sess->ids_waitq, !atomic_read(&sess->ids_inflight));
-}
-
-
static void rtrs_srv_reg_mr_done(struct ib_cq *cq, struct ib_wc *wc)
{
struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
@@ -1479,8 +1485,15 @@ static void rtrs_srv_close_work(struct work_struct *work)
rdma_disconnect(con->c.cm_id);
ib_drain_qp(con->c.qp);
}
- /* Wait for all inflights */
- rtrs_srv_wait_ops_ids(sess);
+
+ /*
+ * Degrade ref count to the usual model with a single shared
+ * atomic_t counter
+ */
+ percpu_ref_kill(&sess->ids_inflight_ref);
+
+ /* Wait for all completion */
+ wait_for_completion(&sess->complete_done);
/* Notify upper layer if we are the last path */
rtrs_srv_sess_down(sess);
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.h b/drivers/infiniband/ulp/rtrs/rtrs-srv.h
index 9543ae19996c..f8da2e3f0bda 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.h
@@ -81,8 +81,8 @@ struct rtrs_srv_sess {
spinlock_t state_lock;
int cur_cq_vector;
struct rtrs_srv_op **ops_ids;
- atomic_t ids_inflight;
- wait_queue_head_t ids_waitq;
+ struct percpu_ref ids_inflight_ref;
+ struct completion complete_done;
struct rtrs_srv_mr *mrs;
unsigned int mrs_num;
dma_addr_t *dma_addr;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCHv2 for-next 13/19] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight
2021-05-17 9:18 ` [PATCHv2 for-next 13/19] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight Gioh Kim
@ 2021-05-25 20:15 ` Jason Gunthorpe
2021-05-26 9:16 ` Haris Iqbal
0 siblings, 1 reply; 28+ messages in thread
From: Jason Gunthorpe @ 2021-05-25 20:15 UTC (permalink / raw)
To: Gioh Kim
Cc: linux-rdma, bvanassche, leon, dledford, haris.iqbal, jinpu.wang,
Md Haris Iqbal
On Mon, May 17, 2021 at 11:18:37AM +0200, Gioh Kim wrote:
> From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
>
> ids_inflight is used to track the inflight IOs. But the use of atomic_t
> variable can cause performance drops and can also become a performance
> bottleneck.
>
> This commit replaces the use of atomic_t with a percpu_ref structure. The
> advantage it offers is, it doesn't check if the reference has fallen to 0,
> until the user explicitly signals it to; and that is done by the
> percpu_ref_kill() function call. After that, the percpu_ref structure
> behaves like an atomic_t and for every put call, checks whether the
> reference has fallen to 0 or not.
>
> rtrs_srv_stats_rdma_to_str shows the count of ids_inflight as 0
> for user-mode tools not to be confused.
>
> Fixes: 9cb837480424e ("RDMA/rtrs: server: main functionality")
> Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c | 12 +++---
> drivers/infiniband/ulp/rtrs/rtrs-srv.c | 43 +++++++++++++-------
> drivers/infiniband/ulp/rtrs/rtrs-srv.h | 4 +-
> 3 files changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> index e102b1368d0c..df1d7d6b1884 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> @@ -27,12 +27,10 @@ ssize_t rtrs_srv_stats_rdma_to_str(struct rtrs_srv_stats *stats,
> char *page, size_t len)
> {
> struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> - struct rtrs_srv_sess *sess = stats->sess;
>
> - return scnprintf(page, len, "%lld %lld %lld %lld %u\n",
> - (s64)atomic64_read(&r->dir[READ].cnt),
> - (s64)atomic64_read(&r->dir[READ].size_total),
> - (s64)atomic64_read(&r->dir[WRITE].cnt),
> - (s64)atomic64_read(&r->dir[WRITE].size_total),
> - atomic_read(&sess->ids_inflight));
> + return sysfs_emit(page, "%lld %lld %lld %lldn\n",
> + (s64)atomic64_read(&r->dir[READ].cnt),
> + (s64)atomic64_read(&r->dir[READ].size_total),
> + (s64)atomic64_read(&r->dir[WRITE].cnt),
> + (s64)atomic64_read(&r->dir[WRITE].size_total));
> }
This seems like an unrelated hunk
Jason
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 for-next 13/19] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight
2021-05-25 20:15 ` Jason Gunthorpe
@ 2021-05-26 9:16 ` Haris Iqbal
2021-05-26 9:38 ` Jinpu Wang
0 siblings, 1 reply; 28+ messages in thread
From: Haris Iqbal @ 2021-05-26 9:16 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Gioh Kim, linux-rdma, bvanassche, Leon Romanovsky, Doug Ledford,
Jinpu Wang, Md Haris Iqbal
On Tue, May 25, 2021 at 10:15 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Mon, May 17, 2021 at 11:18:37AM +0200, Gioh Kim wrote:
> > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> >
> > ids_inflight is used to track the inflight IOs. But the use of atomic_t
> > variable can cause performance drops and can also become a performance
> > bottleneck.
> >
> > This commit replaces the use of atomic_t with a percpu_ref structure. The
> > advantage it offers is, it doesn't check if the reference has fallen to 0,
> > until the user explicitly signals it to; and that is done by the
> > percpu_ref_kill() function call. After that, the percpu_ref structure
> > behaves like an atomic_t and for every put call, checks whether the
> > reference has fallen to 0 or not.
> >
> > rtrs_srv_stats_rdma_to_str shows the count of ids_inflight as 0
> > for user-mode tools not to be confused.
> >
> > Fixes: 9cb837480424e ("RDMA/rtrs: server: main functionality")
> > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > ---
> > drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c | 12 +++---
> > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 43 +++++++++++++-------
> > drivers/infiniband/ulp/rtrs/rtrs-srv.h | 4 +-
> > 3 files changed, 35 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > index e102b1368d0c..df1d7d6b1884 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > @@ -27,12 +27,10 @@ ssize_t rtrs_srv_stats_rdma_to_str(struct rtrs_srv_stats *stats,
> > char *page, size_t len)
> > {
> > struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> > - struct rtrs_srv_sess *sess = stats->sess;
> >
> > - return scnprintf(page, len, "%lld %lld %lld %lld %u\n",
> > - (s64)atomic64_read(&r->dir[READ].cnt),
> > - (s64)atomic64_read(&r->dir[READ].size_total),
> > - (s64)atomic64_read(&r->dir[WRITE].cnt),
> > - (s64)atomic64_read(&r->dir[WRITE].size_total),
> > - atomic_read(&sess->ids_inflight));
> > + return sysfs_emit(page, "%lld %lld %lld %lldn\n",
> > + (s64)atomic64_read(&r->dir[READ].cnt),
> > + (s64)atomic64_read(&r->dir[READ].size_total),
> > + (s64)atomic64_read(&r->dir[WRITE].cnt),
> > + (s64)atomic64_read(&r->dir[WRITE].size_total));
> > }
>
> This seems like an unrelated hunk
Previous to the commit, ids_inflight was an atomic variable, and hence
was read and printed along with other rtrs_srv stats. With this
commit, since ids_inflight is changed to percpu_ref, we removed it
being printed in rtrs_srv stats. Its related to the commit.
>
> Jason
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 for-next 13/19] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight
2021-05-26 9:16 ` Haris Iqbal
@ 2021-05-26 9:38 ` Jinpu Wang
2021-05-27 9:25 ` Haris Iqbal
0 siblings, 1 reply; 28+ messages in thread
From: Jinpu Wang @ 2021-05-26 9:38 UTC (permalink / raw)
To: Haris Iqbal
Cc: Jason Gunthorpe, Gioh Kim, RDMA mailing list, Bart Van Assche,
Leon Romanovsky, Doug Ledford, Md Haris Iqbal
On Wed, May 26, 2021 at 11:17 AM Haris Iqbal <haris.iqbal@ionos.com> wrote:
>
> On Tue, May 25, 2021 at 10:15 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Mon, May 17, 2021 at 11:18:37AM +0200, Gioh Kim wrote:
> > > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> > >
> > > ids_inflight is used to track the inflight IOs. But the use of atomic_t
> > > variable can cause performance drops and can also become a performance
> > > bottleneck.
> > >
> > > This commit replaces the use of atomic_t with a percpu_ref structure. The
> > > advantage it offers is, it doesn't check if the reference has fallen to 0,
> > > until the user explicitly signals it to; and that is done by the
> > > percpu_ref_kill() function call. After that, the percpu_ref structure
> > > behaves like an atomic_t and for every put call, checks whether the
> > > reference has fallen to 0 or not.
> > >
> > > rtrs_srv_stats_rdma_to_str shows the count of ids_inflight as 0
> > > for user-mode tools not to be confused.
> > >
> > > Fixes: 9cb837480424e ("RDMA/rtrs: server: main functionality")
> > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > > ---
> > > drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c | 12 +++---
> > > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 43 +++++++++++++-------
> > > drivers/infiniband/ulp/rtrs/rtrs-srv.h | 4 +-
> > > 3 files changed, 35 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > > index e102b1368d0c..df1d7d6b1884 100644
> > > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > > @@ -27,12 +27,10 @@ ssize_t rtrs_srv_stats_rdma_to_str(struct rtrs_srv_stats *stats,
> > > char *page, size_t len)
> > > {
> > > struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> > > - struct rtrs_srv_sess *sess = stats->sess;
> > >
> > > - return scnprintf(page, len, "%lld %lld %lld %lld %u\n",
> > > - (s64)atomic64_read(&r->dir[READ].cnt),
> > > - (s64)atomic64_read(&r->dir[READ].size_total),
> > > - (s64)atomic64_read(&r->dir[WRITE].cnt),
> > > - (s64)atomic64_read(&r->dir[WRITE].size_total),
> > > - atomic_read(&sess->ids_inflight));
> > > + return sysfs_emit(page, "%lld %lld %lld %lldn\n",
> > > + (s64)atomic64_read(&r->dir[READ].cnt),
> > > + (s64)atomic64_read(&r->dir[READ].size_total),
> > > + (s64)atomic64_read(&r->dir[WRITE].cnt),
> > > + (s64)atomic64_read(&r->dir[WRITE].size_total));
> > > }
> >
> > This seems like an unrelated hunk
>
> Previous to the commit, ids_inflight was an atomic variable, and hence
> was read and printed along with other rtrs_srv stats. With this
> commit, since ids_inflight is changed to percpu_ref, we removed it
> being printed in rtrs_srv stats. Its related to the commit.
I think Jason meant the sysfs_emit conversion should be in a separate patch.
>
>
> >
> > Jason
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 for-next 13/19] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight
2021-05-26 9:38 ` Jinpu Wang
@ 2021-05-27 9:25 ` Haris Iqbal
0 siblings, 0 replies; 28+ messages in thread
From: Haris Iqbal @ 2021-05-27 9:25 UTC (permalink / raw)
To: Jinpu Wang
Cc: Jason Gunthorpe, Gioh Kim, RDMA mailing list, Bart Van Assche,
Leon Romanovsky, Doug Ledford, Md Haris Iqbal
On Wed, May 26, 2021 at 11:38 AM Jinpu Wang <jinpu.wang@ionos.com> wrote:
>
> On Wed, May 26, 2021 at 11:17 AM Haris Iqbal <haris.iqbal@ionos.com> wrote:
> >
> > On Tue, May 25, 2021 at 10:15 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> > >
> > > On Mon, May 17, 2021 at 11:18:37AM +0200, Gioh Kim wrote:
> > > > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> > > >
> > > > ids_inflight is used to track the inflight IOs. But the use of atomic_t
> > > > variable can cause performance drops and can also become a performance
> > > > bottleneck.
> > > >
> > > > This commit replaces the use of atomic_t with a percpu_ref structure. The
> > > > advantage it offers is, it doesn't check if the reference has fallen to 0,
> > > > until the user explicitly signals it to; and that is done by the
> > > > percpu_ref_kill() function call. After that, the percpu_ref structure
> > > > behaves like an atomic_t and for every put call, checks whether the
> > > > reference has fallen to 0 or not.
> > > >
> > > > rtrs_srv_stats_rdma_to_str shows the count of ids_inflight as 0
> > > > for user-mode tools not to be confused.
> > > >
> > > > Fixes: 9cb837480424e ("RDMA/rtrs: server: main functionality")
> > > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > > > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > > > ---
> > > > drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c | 12 +++---
> > > > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 43 +++++++++++++-------
> > > > drivers/infiniband/ulp/rtrs/rtrs-srv.h | 4 +-
> > > > 3 files changed, 35 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > > > index e102b1368d0c..df1d7d6b1884 100644
> > > > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > > > @@ -27,12 +27,10 @@ ssize_t rtrs_srv_stats_rdma_to_str(struct rtrs_srv_stats *stats,
> > > > char *page, size_t len)
> > > > {
> > > > struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> > > > - struct rtrs_srv_sess *sess = stats->sess;
> > > >
> > > > - return scnprintf(page, len, "%lld %lld %lld %lld %u\n",
> > > > - (s64)atomic64_read(&r->dir[READ].cnt),
> > > > - (s64)atomic64_read(&r->dir[READ].size_total),
> > > > - (s64)atomic64_read(&r->dir[WRITE].cnt),
> > > > - (s64)atomic64_read(&r->dir[WRITE].size_total),
> > > > - atomic_read(&sess->ids_inflight));
> > > > + return sysfs_emit(page, "%lld %lld %lld %lldn\n",
> > > > + (s64)atomic64_read(&r->dir[READ].cnt),
> > > > + (s64)atomic64_read(&r->dir[READ].size_total),
> > > > + (s64)atomic64_read(&r->dir[WRITE].cnt),
> > > > + (s64)atomic64_read(&r->dir[WRITE].size_total));
> > > > }
> > >
> > > This seems like an unrelated hunk
> >
> > Previous to the commit, ids_inflight was an atomic variable, and hence
> > was read and printed along with other rtrs_srv stats. With this
> > commit, since ids_inflight is changed to percpu_ref, we removed it
> > being printed in rtrs_srv stats. Its related to the commit.
> I think Jason meant the sysfs_emit conversion should be in a separate patch.
Okay. I missed that.
Thanks; will send the updated patches.
> >
> >
> > >
> > > Jason
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCHv2 for-next 14/19] RDMA/rtrs: Do not reset hb_missed_max after re-connection
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (12 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 13/19] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 15/19] RDMA/rtrs-srv: Duplicated session name is not allowed Gioh Kim
` (4 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Gioh Kim, Gioh Kim
From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
When re-connecting, it resets hb_missed_max to 0.
Before the first re-connecting, client will trigger re-connection
when it gets hb-ack more than 5 times. But after the first
re-connecting, clients will do re-connection whenever it does
not get hb-ack because hb_missed_max is 0.
There is no need to reset hb_missed_max when re-connecting.
hb_missed_max should be kept until closing the session.
Fixes: c0894b3ea69d3 ("RDMA/rtrs: core: lib functions shared between client and server modules")
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c
index a7847282a2eb..4e602e40f623 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs.c
@@ -376,7 +376,6 @@ void rtrs_stop_hb(struct rtrs_sess *sess)
{
cancel_delayed_work_sync(&sess->hb_dwork);
sess->hb_missed_cnt = 0;
- sess->hb_missed_max = 0;
}
EXPORT_SYMBOL_GPL(rtrs_stop_hb);
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 15/19] RDMA/rtrs-srv: Duplicated session name is not allowed
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (13 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 14/19] RDMA/rtrs: Do not reset hb_missed_max after re-connection Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 16/19] RDMA/rtrs-srv: Fix memory leak of unfreed rtrs_srv_stats object Gioh Kim
` (3 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Gioh Kim, Gioh Kim, Aleksei Marov
From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
If two clients try to use the same session name, rtrs-server generates a
kernel error that it failed to create the sysfs because the filename
is duplicated.
This patch adds code to check if there already exists the same session
name with the different UUID. If a client tries to add more session,
it sends the UUID and the session name. Therefore it is ok if there is
already same session name with the same UUID. The rtrs-server must fail
only-if there is the same session name with the different UUID.
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Aleksei Marov <aleksei.marov@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 42 +++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index f66f2be9f519..0e1e303dcbee 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -753,7 +753,40 @@ static void rtrs_srv_sess_down(struct rtrs_srv_sess *sess)
mutex_unlock(&srv->paths_ev_mutex);
}
+static bool exist_sessname(struct rtrs_srv_ctx *ctx,
+ const char *sessname, const uuid_t *path_uuid)
+{
+ struct rtrs_srv *srv;
+ struct rtrs_srv_sess *sess;
+ bool found = false;
+
+ mutex_lock(&ctx->srv_mutex);
+ list_for_each_entry(srv, &ctx->srv_list, ctx_list) {
+ mutex_lock(&srv->paths_mutex);
+
+ /* when a client with same uuid and same sessname tried to add a path */
+ if (uuid_equal(&srv->paths_uuid, path_uuid)) {
+ mutex_unlock(&srv->paths_mutex);
+ continue;
+ }
+
+ list_for_each_entry(sess, &srv->paths_list, s.entry) {
+ if (strlen(sess->s.sessname) == strlen(sessname) &&
+ !strcmp(sess->s.sessname, sessname)) {
+ found = true;
+ break;
+ }
+ }
+ mutex_unlock(&srv->paths_mutex);
+ if (found)
+ break;
+ }
+ mutex_unlock(&ctx->srv_mutex);
+ return found;
+}
+
static int post_recv_sess(struct rtrs_srv_sess *sess);
+static int rtrs_rdma_do_reject(struct rdma_cm_id *cm_id, int errno);
static int process_info_req(struct rtrs_srv_con *con,
struct rtrs_msg_info_req *msg)
@@ -772,10 +805,17 @@ static int process_info_req(struct rtrs_srv_con *con,
rtrs_err(s, "post_recv_sess(), err: %d\n", err);
return err;
}
+
+ if (exist_sessname(sess->srv->ctx,
+ msg->sessname, &sess->srv->paths_uuid)) {
+ rtrs_err(s, "sessname is duplicated: %s\n", msg->sessname);
+ return -EPERM;
+ }
+ strscpy(sess->s.sessname, msg->sessname, sizeof(sess->s.sessname));
+
rwr = kcalloc(sess->mrs_num, sizeof(*rwr), GFP_KERNEL);
if (unlikely(!rwr))
return -ENOMEM;
- strscpy(sess->s.sessname, msg->sessname, sizeof(sess->s.sessname));
tx_sz = sizeof(*rsp);
tx_sz += sizeof(rsp->desc[0]) * sess->mrs_num;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 16/19] RDMA/rtrs-srv: Fix memory leak of unfreed rtrs_srv_stats object
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (14 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 15/19] RDMA/rtrs-srv: Duplicated session name is not allowed Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 17/19] RDMA/rtrs-srv: Fix memory leak when having multiple sessions Gioh Kim
` (2 subsequent siblings)
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Gioh Kim, Gioh Kim
From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
When closing a session, currently the rtrs_srv_stats object in the
closing session is freed by kobject release. But if it failed
to create a session by various reasons, it must free the rtrs_srv_stats
object directly because kobject is not created yet.
This problem is found by kmemleak as below:
1. One client machine maps /dev/nullb0 with session name 'bla':
root@test1:~# echo "sessname=bla path=ip:192.168.122.190 \
device_path=/dev/nullb0" > /sys/devices/virtual/rnbd-client/ctl/map_device
2. Another machine failed to create a session with the same name 'bla':
root@test2:~# echo "sessname=bla path=ip:192.168.122.190 \
device_path=/dev/nullb1" > /sys/devices/virtual/rnbd-client/ctl/map_device
-bash: echo: write error: Connection reset by peer
3. The kmemleak on server machine reported an error:
unreferenced object 0xffff888033cdc800 (size 128):
comm "kworker/2:1", pid 83, jiffies 4295086585 (age 2508.680s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000a72903b2>] __alloc_sess+0x1d4/0x1250 [rtrs_server]
[<00000000d1e5321e>] rtrs_srv_rdma_cm_handler+0xc31/0xde0 [rtrs_server]
[<00000000bb2f6e7e>] cma_ib_req_handler+0xdc5/0x2b50 [rdma_cm]
[<00000000e896235d>] cm_process_work+0x2d/0x100 [ib_cm]
[<00000000b6866c5f>] cm_req_handler+0x11bc/0x1c40 [ib_cm]
[<000000005f5dd9aa>] cm_work_handler+0xe65/0x3cf2 [ib_cm]
[<00000000610151e7>] process_one_work+0x4bc/0x980
[<00000000541e0f77>] worker_thread+0x78/0x5c0
[<00000000423898ca>] kthread+0x191/0x1e0
[<000000005a24b239>] ret_from_fork+0x3a/0x50
Fixes: 39c2d639ca183 ("RDMA/rtrs-srv: Set .release function for rtrs srv device during device init")
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 0e1e303dcbee..879e6dbca86d 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1503,6 +1503,7 @@ static void free_sess(struct rtrs_srv_sess *sess)
kobject_del(&sess->kobj);
kobject_put(&sess->kobj);
} else {
+ kfree(sess->stats);
kfree(sess);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 17/19] RDMA/rtrs-srv: Fix memory leak when having multiple sessions
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (15 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 16/19] RDMA/rtrs-srv: Fix memory leak of unfreed rtrs_srv_stats object Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 18/19] RDMA/rtrs-clt: Check if the queue_depth has changed during a reconnection Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 19/19] RDMA/rtrs-clt: Fix memory leak of not-freed sess->stats and stats->pcpu_stats Gioh Kim
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Jack Wang, Gioh Kim
From: Jack Wang <jinpu.wang@cloud.ionos.com>
Gioh notice memory leak below
unreferenced object 0xffff8880acda2000 (size 2048):
comm "kworker/4:1", pid 77, jiffies 4295062871 (age 1270.730s)
hex dump (first 32 bytes):
00 20 da ac 80 88 ff ff 00 20 da ac 80 88 ff ff . ....... ......
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000e85d85b5>] rtrs_srv_rdma_cm_handler+0x8e5/0xa90 [rtrs_server]
[<00000000e31a988a>] cma_ib_req_handler+0xdc5/0x2b50 [rdma_cm]
[<000000000eb02c5b>] cm_process_work+0x2d/0x100 [ib_cm]
[<00000000e1650ca9>] cm_req_handler+0x11bc/0x1c40 [ib_cm]
[<000000009c28818b>] cm_work_handler+0xe65/0x3cf2 [ib_cm]
[<000000002b53eaa1>] process_one_work+0x4bc/0x980
[<00000000da3499fb>] worker_thread+0x78/0x5c0
[<00000000167127a4>] kthread+0x191/0x1e0
[<0000000060802104>] ret_from_fork+0x3a/0x50
unreferenced object 0xffff88806d595d90 (size 8):
comm "kworker/4:1H", pid 131, jiffies 4295062972 (age 1269.720s)
hex dump (first 8 bytes):
62 6c 61 00 6b 6b 6b a5 bla.kkk.
backtrace:
[<000000004447d253>] kstrdup+0x2e/0x60
[<0000000047259793>] kobject_set_name_vargs+0x2f/0xb0
[<00000000c2ee3bc8>] dev_set_name+0xab/0xe0
[<000000002b6bdfb1>] rtrs_srv_create_sess_files+0x260/0x290 [rtrs_server]
[<0000000075d87bd7>] rtrs_srv_info_req_done+0x71b/0x960 [rtrs_server]
[<00000000ccdf1bb5>] __ib_process_cq+0x94/0x100 [ib_core]
[<00000000cbcb60cb>] ib_cq_poll_work+0x32/0xc0 [ib_core]
[<000000002b53eaa1>] process_one_work+0x4bc/0x980
[<00000000da3499fb>] worker_thread+0x78/0x5c0
[<00000000167127a4>] kthread+0x191/0x1e0
[<0000000060802104>] ret_from_fork+0x3a/0x50
unreferenced object 0xffff88806d6bb100 (size 256):
comm "kworker/4:1H", pid 131, jiffies 4295062972 (age 1269.720s)
hex dump (first 32 bytes):
00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N..........
ff ff ff ff ff ff ff ff 00 59 4d 86 ff ff ff ff .........YM.....
backtrace:
[<00000000a18a11e4>] device_add+0x74d/0xa00
[<00000000a915b95f>] rtrs_srv_create_sess_files.cold+0x49/0x1fe [rtrs_server]
[<0000000075d87bd7>] rtrs_srv_info_req_done+0x71b/0x960 [rtrs_server]
[<00000000ccdf1bb5>] __ib_process_cq+0x94/0x100 [ib_core]
[<00000000cbcb60cb>] ib_cq_poll_work+0x32/0xc0 [ib_core]
[<000000002b53eaa1>] process_one_work+0x4bc/0x980
[<00000000da3499fb>] worker_thread+0x78/0x5c0
[<00000000167127a4>] kthread+0x191/0x1e0
[<0000000060802104>] ret_from_fork+0x3a/0x50
The problem is we increase device refcount by get_device in
process_info_req for each path, but only does put_deice for last path,
which lead to memory leak.
To fix it, it also calls put_device when dev_ref is not 0.
Fixes: e2853c49477d1 ("RDMA/rtrs-srv-sysfs: fix missing put_device")
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
index a9288175fbb5..20efd44297fb 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
@@ -208,6 +208,7 @@ rtrs_srv_destroy_once_sysfs_root_folders(struct rtrs_srv_sess *sess)
device_del(&srv->dev);
put_device(&srv->dev);
} else {
+ put_device(&srv->dev);
mutex_unlock(&srv->paths_mutex);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 18/19] RDMA/rtrs-clt: Check if the queue_depth has changed during a reconnection
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (16 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 17/19] RDMA/rtrs-srv: Fix memory leak when having multiple sessions Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
2021-05-17 9:18 ` [PATCHv2 for-next 19/19] RDMA/rtrs-clt: Fix memory leak of not-freed sess->stats and stats->pcpu_stats Gioh Kim
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Md Haris Iqbal, Gioh Kim
From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
The queue_depth is a module parameter for rtrs_server. It is used on the
client side to determing the queue_depth of the request queue for the RNBD
virtual block device.
During a reconnection event for an already mapped device, in case the
rtrs_server module queue_depth has changed, fail the reconnect attempt.
Also stop further auto reconnection attempts. A manual reconnect via
sysfs has to be triggerred.
Fixes: 6a98d71daea18 ("RDMA/rtrs: client: main functionality")
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index a3d20a5c5112..1bfa3f86c430 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1770,7 +1770,20 @@ static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
queue_depth);
return -ECONNRESET;
}
- if (!sess->rbufs || sess->queue_depth < queue_depth) {
+
+ if (sess->queue_depth > 0 && queue_depth != sess->queue_depth) {
+ rtrs_err(clt, "Error: queue depth changed\n");
+
+ /*
+ * Stop any more reconnection attempts
+ */
+ sess->reconnect_attempts = -1;
+ rtrs_err(clt,
+ "Disabling auto-reconnect. Trigger a manual reconnect after issue is resolved\n");
+ return -ECONNRESET;
+ }
+
+ if (!sess->rbufs) {
kfree(sess->rbufs);
sess->rbufs = kcalloc(queue_depth, sizeof(*sess->rbufs),
GFP_KERNEL);
@@ -1784,7 +1797,7 @@ static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
sess->chunk_size = sess->max_io_size + sess->max_hdr_size;
/*
- * Global queue depth and IO size is always a minimum.
+ * Global IO size is always a minimum.
* If while a reconnection server sends us a value a bit
* higher - client does not care and uses cached minimum.
*
@@ -1792,8 +1805,7 @@ static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
* connections in parallel, use lock.
*/
mutex_lock(&clt->paths_mutex);
- clt->queue_depth = min_not_zero(sess->queue_depth,
- clt->queue_depth);
+ clt->queue_depth = sess->queue_depth;
clt->max_io_size = min_not_zero(sess->max_io_size,
clt->max_io_size);
mutex_unlock(&clt->paths_mutex);
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCHv2 for-next 19/19] RDMA/rtrs-clt: Fix memory leak of not-freed sess->stats and stats->pcpu_stats
2021-05-17 9:18 [PATCHv2 for-next 00/19] Misc update for rtrs Gioh Kim
` (17 preceding siblings ...)
2021-05-17 9:18 ` [PATCHv2 for-next 18/19] RDMA/rtrs-clt: Check if the queue_depth has changed during a reconnection Gioh Kim
@ 2021-05-17 9:18 ` Gioh Kim
18 siblings, 0 replies; 28+ messages in thread
From: Gioh Kim @ 2021-05-17 9:18 UTC (permalink / raw)
To: linux-rdma
Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
Gioh Kim, Gioh Kim
From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
sess->stats and sess->stats->pcpu_stats objects are freed
when sysfs entry is removed. If something wrong happens and
session is closed before sysfs entry is created,
sess->stats and sess->stats->pcpu_stats objects are not freed.
This patch adds freeing of them at three places:
1. When client uses wrong address and session creation fails.
2. When client fails to create a sysfs entry.
3. When client adds wrong address via sysfs add_path.
Fixes: 215378b838df0 ("RDMA/rtrs: client: sysfs interface functions")
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 1bfa3f86c430..2323c9ddae97 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2759,6 +2759,8 @@ struct rtrs_clt *rtrs_clt_open(struct rtrs_clt_ops *ops,
if (err) {
list_del_rcu(&sess->s.entry);
rtrs_clt_close_conns(sess, true);
+ free_percpu(sess->stats->pcpu_stats);
+ kfree(sess->stats);
free_sess(sess);
goto close_all_sess;
}
@@ -2767,6 +2769,8 @@ struct rtrs_clt *rtrs_clt_open(struct rtrs_clt_ops *ops,
if (err) {
list_del_rcu(&sess->s.entry);
rtrs_clt_close_conns(sess, true);
+ free_percpu(sess->stats->pcpu_stats);
+ kfree(sess->stats);
free_sess(sess);
goto close_all_sess;
}
@@ -3017,6 +3021,8 @@ int rtrs_clt_create_path_from_sysfs(struct rtrs_clt *clt,
close_sess:
rtrs_clt_remove_path_from_arr(sess);
rtrs_clt_close_conns(sess, true);
+ free_percpu(sess->stats->pcpu_stats);
+ kfree(sess->stats);
free_sess(sess);
return err;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread