On 08/11/2015 07:42 AM, Sagi Grimberg wrote:
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
> b/drivers/infiniband/ulp/srp/ib_srp.c
> index 3a1514c..b220856 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -546,6 +546,17 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
> if (ret)
> goto err_qp;
>
> + if (ch->qp)
> + srp_destroy_qp(ch);
> + if (ch->recv_cq)
> + ib_destroy_cq(ch->recv_cq);
> + if (ch->send_cq)
> + ib_destroy_cq(ch->send_cq);
> +
> + ch->qp = qp;
> + ch->recv_cq = recv_cq;
> + ch->send_cq = send_cq;
> +
> if (dev->use_fast_reg && dev->has_fr) {
> fr_pool = srp_alloc_fr_pool(target);
> if (IS_ERR(fr_pool)) {
> @@ -570,17 +581,6 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
> ch->fmr_pool = fmr_pool;
> }
>
> - if (ch->qp)
> - srp_destroy_qp(ch);
> - if (ch->recv_cq)
> - ib_destroy_cq(ch->recv_cq);
> -
> - ch->qp = qp;
> - ch->recv_cq = recv_cq;
> - ch->send_cq = send_cq;
> -
> kfree(init_attr);
> return 0;
>
> Sorry for the mixup. Does this patch make more sense?
On second thought ... with your patch, if the "goto err_qp" branch in
srp_create_ch_ib() is taken upon return ch->qp, ch->recv_cq and
ch->send_cq will be dangling pointers. That will have bad consequences
in the subsequent srp_free_ch_ib() call. How about replacing the above
patch with the (untested) patch below ?
Thanks,
Bart.
[PATCH] IB/srp: Avoid that a completion during reconnect causes a crash
Untested.
---
drivers/infiniband/ulp/srp/ib_srp.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 2968b7b..1f9ed68 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -554,9 +554,6 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
"FR pool allocation failed (%d)\n", ret);
goto err_qp;
}
- if (ch->fr_pool)
- srp_destroy_fr_pool(ch->fr_pool);
- ch->fr_pool = fr_pool;
} else if (!dev->use_fast_reg && dev->has_fmr) {
fmr_pool = srp_alloc_fmr_pool(target);
if (IS_ERR(fmr_pool)) {
@@ -565,9 +562,6 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
"FMR pool allocation failed (%d)\n", ret);
goto err_qp;
}
- if (ch->fmr_pool)
- ib_destroy_fmr_pool(ch->fmr_pool);
- ch->fmr_pool = fmr_pool;
}
if (ch->qp)
@@ -577,6 +571,16 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
if (ch->send_cq)
ib_destroy_cq(ch->send_cq);
+ if (dev->use_fast_reg && dev->has_fr) {
+ if (ch->fr_pool)
+ srp_destroy_fr_pool(ch->fr_pool);
+ ch->fr_pool = fr_pool;
+ } else if (!dev->use_fast_reg && dev->has_fmr) {
+ if (ch->fmr_pool)
+ ib_destroy_fmr_pool(ch->fmr_pool);
+ ch->fmr_pool = fmr_pool;
+ }
+
ch->qp = qp;
ch->recv_cq = recv_cq;
ch->send_cq = send_cq;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
