diff mbox

[RFC] iser-target: avoid reinitializing rdma contexts for isert commands

Message ID 1511893687-29298-1-git-send-email-bharat@chelsio.com (mailing list archive)
State New, archived
Headers show

Commit Message

Potnuri Bharat Teja Nov. 28, 2017, 6:28 p.m. UTC
isert commands that failed during isert_rdma_rw_ctx_post() are queued to
Queue-Full(QF) queue and are scheduled to be reposted during queue-full
queue processing. During this reposting, the rdma contexts are initialised
again in isert_rdma_rw_ctx_post(), which is leaking significant memory.

unreferenced object 0xffff8830201d9640 (size 64):
  comm "kworker/0:2", pid 195, jiffies 4295374851 (age 4528.436s)
  hex dump (first 32 bytes):
    00 60 8b cb 2e 00 00 00 00 10 00 00 00 00 00 00  .`..............
    00 90 e3 cb 2e 00 00 00 00 10 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff8170711e>] kmemleak_alloc+0x4e/0xb0
    [<ffffffff811f8ba5>] __kmalloc+0x125/0x2b0
    [<ffffffffa046b24f>] rdma_rw_ctx_init+0x15f/0x6f0 [ib_core]
    [<ffffffffa07ab644>] isert_rdma_rw_ctx_post+0xc4/0x3c0 [ib_isert]
    [<ffffffffa07ad972>] isert_put_datain+0x112/0x1c0 [ib_isert]
    [<ffffffffa07dddce>] lio_queue_data_in+0x2e/0x30 [iscsi_target_mod]
    [<ffffffffa076c322>] target_qf_do_work+0x2b2/0x4b0 [target_core_mod]
    [<ffffffff81080c3b>] process_one_work+0x1db/0x5d0
    [<ffffffff8108107d>] worker_thread+0x4d/0x3e0
    [<ffffffff81088667>] kthread+0x117/0x150
    [<ffffffff81713fa7>] ret_from_fork+0x27/0x40
    [<ffffffffffffffff>] 0xffffffffffffffff

Here is patch to use the older rdma contexts while reposting
the isert commands intead of reinitialising them.

Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
---
 drivers/infiniband/ulp/isert/ib_isert.c | 7 +++++++
 drivers/infiniband/ulp/isert/ib_isert.h | 1 +
 2 files changed, 8 insertions(+)

Comments

Sagi Grimberg Nov. 29, 2017, 10:50 p.m. UTC | #1
Hi Bharat,

Thanks for the patch, comment below.

> isert commands that failed during isert_rdma_rw_ctx_post() are queued to
> Queue-Full(QF) queue and are scheduled to be reposted during queue-full
> queue processing. During this reposting, the rdma contexts are initialised
> again in isert_rdma_rw_ctx_post(), which is leaking significant memory.
> 
> unreferenced object 0xffff8830201d9640 (size 64):
>    comm "kworker/0:2", pid 195, jiffies 4295374851 (age 4528.436s)
>    hex dump (first 32 bytes):
>      00 60 8b cb 2e 00 00 00 00 10 00 00 00 00 00 00  .`..............
>      00 90 e3 cb 2e 00 00 00 00 10 00 00 00 00 00 00  ................
>    backtrace:
>      [<ffffffff8170711e>] kmemleak_alloc+0x4e/0xb0
>      [<ffffffff811f8ba5>] __kmalloc+0x125/0x2b0
>      [<ffffffffa046b24f>] rdma_rw_ctx_init+0x15f/0x6f0 [ib_core]
>      [<ffffffffa07ab644>] isert_rdma_rw_ctx_post+0xc4/0x3c0 [ib_isert]
>      [<ffffffffa07ad972>] isert_put_datain+0x112/0x1c0 [ib_isert]
>      [<ffffffffa07dddce>] lio_queue_data_in+0x2e/0x30 [iscsi_target_mod]
>      [<ffffffffa076c322>] target_qf_do_work+0x2b2/0x4b0 [target_core_mod]
>      [<ffffffff81080c3b>] process_one_work+0x1db/0x5d0
>      [<ffffffff8108107d>] worker_thread+0x4d/0x3e0
>      [<ffffffff81088667>] kthread+0x117/0x150
>      [<ffffffff81713fa7>] ret_from_fork+0x27/0x40
>      [<ffffffffffffffff>] 0xffffffffffffffff
> 
> Here is patch to use the older rdma contexts while reposting
> the isert commands intead of reinitialising them.
> 
> Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> ---
>   drivers/infiniband/ulp/isert/ib_isert.c | 7 +++++++
>   drivers/infiniband/ulp/isert/ib_isert.h | 1 +
>   2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> index 720dfb3a1ac2..fd55163801a3 100644
> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> @@ -2123,6 +2123,9 @@ isert_rdma_rw_ctx_post(struct isert_cmd *cmd, struct isert_conn *conn,
>   	u32 rkey, offset;
>   	int ret;
>   
> +	if (cmd->ctx_init_done)
> +		goto rdma_ctx_post;
> +
>   	if (dir == DMA_FROM_DEVICE) {
>   		addr = cmd->write_va;
>   		rkey = cmd->write_stag;
> @@ -2150,11 +2153,15 @@ isert_rdma_rw_ctx_post(struct isert_cmd *cmd, struct isert_conn *conn,
>   				se_cmd->t_data_sg, se_cmd->t_data_nents,
>   				offset, addr, rkey, dir);
>   	}
> +
>   	if (ret < 0) {
>   		isert_err("Cmd: %p failed to prepare RDMA res\n", cmd);
>   		return ret;
>   	}
>   
> +	cmd->ctx_init_done = true;
> +
> +rdma_ctx_post:
>   	ret = rdma_rw_ctx_post(&cmd->rw, conn->qp, port_num, cqe, chain_wr);
>   	if (ret < 0)
>   		isert_err("Cmd: %p failed to post RDMA res\n", cmd);

I'm not convinced this is sufficient, for reads (isert_put_datain) we
need to skip duplicating the post_recv as well.
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Potnuri Bharat Teja Nov. 30, 2017, 10:34 a.m. UTC | #2
On Thursday, November 11/30/17, 2017 at 04:20:31 +0530, Sagi Grimberg wrote:
> Hi Bharat,
> 
> Thanks for the patch, comment below.
> 
> > isert commands that failed during isert_rdma_rw_ctx_post() are queued to
> > Queue-Full(QF) queue and are scheduled to be reposted during queue-full
> > queue processing. During this reposting, the rdma contexts are initialised
> > again in isert_rdma_rw_ctx_post(), which is leaking significant memory.
> > 
> > unreferenced object 0xffff8830201d9640 (size 64):
> >    comm "kworker/0:2", pid 195, jiffies 4295374851 (age 4528.436s)
> >    hex dump (first 32 bytes):
> >      00 60 8b cb 2e 00 00 00 00 10 00 00 00 00 00 00  .`..............
> >      00 90 e3 cb 2e 00 00 00 00 10 00 00 00 00 00 00  ................
> >    backtrace:
> >      [<ffffffff8170711e>] kmemleak_alloc+0x4e/0xb0
> >      [<ffffffff811f8ba5>] __kmalloc+0x125/0x2b0
> >      [<ffffffffa046b24f>] rdma_rw_ctx_init+0x15f/0x6f0 [ib_core]
> >      [<ffffffffa07ab644>] isert_rdma_rw_ctx_post+0xc4/0x3c0 [ib_isert]
> >      [<ffffffffa07ad972>] isert_put_datain+0x112/0x1c0 [ib_isert]
> >      [<ffffffffa07dddce>] lio_queue_data_in+0x2e/0x30 [iscsi_target_mod]
> >      [<ffffffffa076c322>] target_qf_do_work+0x2b2/0x4b0 [target_core_mod]
> >      [<ffffffff81080c3b>] process_one_work+0x1db/0x5d0
> >      [<ffffffff8108107d>] worker_thread+0x4d/0x3e0
> >      [<ffffffff81088667>] kthread+0x117/0x150
> >      [<ffffffff81713fa7>] ret_from_fork+0x27/0x40
> >      [<ffffffffffffffff>] 0xffffffffffffffff
> > 
> > Here is patch to use the older rdma contexts while reposting
> > the isert commands intead of reinitialising them.
> > 
> > Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> > ---
> >   drivers/infiniband/ulp/isert/ib_isert.c | 7 +++++++
> >   drivers/infiniband/ulp/isert/ib_isert.h | 1 +
> >   2 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> > index 720dfb3a1ac2..fd55163801a3 100644
> > --- a/drivers/infiniband/ulp/isert/ib_isert.c
> > +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> > @@ -2123,6 +2123,9 @@ isert_rdma_rw_ctx_post(struct isert_cmd *cmd, struct isert_conn *conn,
> >   	u32 rkey, offset;
> >   	int ret;
> >   
> > +	if (cmd->ctx_init_done)
> > +		goto rdma_ctx_post;
> > +
> >   	if (dir == DMA_FROM_DEVICE) {
> >   		addr = cmd->write_va;
> >   		rkey = cmd->write_stag;
> > @@ -2150,11 +2153,15 @@ isert_rdma_rw_ctx_post(struct isert_cmd *cmd, struct isert_conn *conn,
> >   				se_cmd->t_data_sg, se_cmd->t_data_nents,
> >   				offset, addr, rkey, dir);
> >   	}
> > +
> >   	if (ret < 0) {
> >   		isert_err("Cmd: %p failed to prepare RDMA res\n", cmd);
> >   		return ret;
> >   	}
> >   
> > +	cmd->ctx_init_done = true;
> > +
> > +rdma_ctx_post:
> >   	ret = rdma_rw_ctx_post(&cmd->rw, conn->qp, port_num, cqe, chain_wr);
> >   	if (ret < 0)
> >   		isert_err("Cmd: %p failed to post RDMA res\n", cmd);
> 
> I'm not convinced this is sufficient, for reads (isert_put_datain) we
> need to skip duplicating the post_recv as well.
Hi Sagi,
Thanks for the review.
The post recv duplication is already handled as a part of "Queue full response
handling" changes, Here is the patch that handles it:
https://patchwork.kernel.org/patch/9639185/

  drivers/infiniband/ulp/isert/ib_isert.c
  -----------------------------------------------
   static int
   isert_post_recv(struct isert_conn *isert_conn, struct iser_rx_desc *rx_desc)
   {
           struct ib_recv_wr *rx_wr_failed, rx_wr;
           int ret;

  >         if (!rx_desc->in_use) {
  >                 /*
  >                  * if the descriptor is not in-use we already reposted it
  >                  * for recv, so just silently return
  >                  */
  >                 return 0;
  >         }
  ------------------------------------------------

above patch avoids reposting the recv WR.

Thanks,
Bharat.


--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sagi Grimberg Dec. 4, 2017, 6:31 p.m. UTC | #3
> Hi Sagi,
> Thanks for the review.
> The post recv duplication is already handled as a part of "Queue full response
> handling" changes, Here is the patch that handles it:
> https://patchwork.kernel.org/patch/9639185/

Right, forgot about that.

So looks fine to me,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Dec. 13, 2017, 5:51 p.m. UTC | #4
On Tue, Nov 28, 2017 at 11:58:07PM +0530, Bharat Potnuri wrote:
> isert commands that failed during isert_rdma_rw_ctx_post() are queued to
> Queue-Full(QF) queue and are scheduled to be reposted during queue-full
> queue processing. During this reposting, the rdma contexts are initialised
> again in isert_rdma_rw_ctx_post(), which is leaking significant memory.
> 
> unreferenced object 0xffff8830201d9640 (size 64):
>   comm "kworker/0:2", pid 195, jiffies 4295374851 (age 4528.436s)
>   hex dump (first 32 bytes):
>     00 60 8b cb 2e 00 00 00 00 10 00 00 00 00 00 00  .`..............
>     00 90 e3 cb 2e 00 00 00 00 10 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff8170711e>] kmemleak_alloc+0x4e/0xb0
>     [<ffffffff811f8ba5>] __kmalloc+0x125/0x2b0
>     [<ffffffffa046b24f>] rdma_rw_ctx_init+0x15f/0x6f0 [ib_core]
>     [<ffffffffa07ab644>] isert_rdma_rw_ctx_post+0xc4/0x3c0 [ib_isert]
>     [<ffffffffa07ad972>] isert_put_datain+0x112/0x1c0 [ib_isert]
>     [<ffffffffa07dddce>] lio_queue_data_in+0x2e/0x30 [iscsi_target_mod]
>     [<ffffffffa076c322>] target_qf_do_work+0x2b2/0x4b0 [target_core_mod]
>     [<ffffffff81080c3b>] process_one_work+0x1db/0x5d0
>     [<ffffffff8108107d>] worker_thread+0x4d/0x3e0
>     [<ffffffff81088667>] kthread+0x117/0x150
>     [<ffffffff81713fa7>] ret_from_fork+0x27/0x40
>     [<ffffffffffffffff>] 0xffffffffffffffff
> 
> Here is patch to use the older rdma contexts while reposting
> the isert commands intead of reinitialising them.
> 
> Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
>  drivers/infiniband/ulp/isert/ib_isert.c | 7 +++++++
>  drivers/infiniband/ulp/isert/ib_isert.h | 1 +
>  2 files changed, 8 insertions(+)
>

So this RFC got Reviewed-by Sagi, is it to be applied now or something
else??

If yes, what tree does it want to go through? I see Nicholas and Doug
have both been historically taking isert patches.

Thanks,
Jason
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Potnuri Bharat Teja Dec. 13, 2017, 6:48 p.m. UTC | #5
On Wednesday, December 12/13/17, 2017 at 23:21:04 +0530, Jason Gunthorpe wrote:
> On Tue, Nov 28, 2017 at 11:58:07PM +0530, Bharat Potnuri wrote:
> > isert commands that failed during isert_rdma_rw_ctx_post() are queued to
> > Queue-Full(QF) queue and are scheduled to be reposted during queue-full
> > queue processing. During this reposting, the rdma contexts are initialised
> > again in isert_rdma_rw_ctx_post(), which is leaking significant memory.
> > 
> > unreferenced object 0xffff8830201d9640 (size 64):
> >   comm "kworker/0:2", pid 195, jiffies 4295374851 (age 4528.436s)
> >   hex dump (first 32 bytes):
> >     00 60 8b cb 2e 00 00 00 00 10 00 00 00 00 00 00  .`..............
> >     00 90 e3 cb 2e 00 00 00 00 10 00 00 00 00 00 00  ................
> >   backtrace:
> >     [<ffffffff8170711e>] kmemleak_alloc+0x4e/0xb0
> >     [<ffffffff811f8ba5>] __kmalloc+0x125/0x2b0
> >     [<ffffffffa046b24f>] rdma_rw_ctx_init+0x15f/0x6f0 [ib_core]
> >     [<ffffffffa07ab644>] isert_rdma_rw_ctx_post+0xc4/0x3c0 [ib_isert]
> >     [<ffffffffa07ad972>] isert_put_datain+0x112/0x1c0 [ib_isert]
> >     [<ffffffffa07dddce>] lio_queue_data_in+0x2e/0x30 [iscsi_target_mod]
> >     [<ffffffffa076c322>] target_qf_do_work+0x2b2/0x4b0 [target_core_mod]
> >     [<ffffffff81080c3b>] process_one_work+0x1db/0x5d0
> >     [<ffffffff8108107d>] worker_thread+0x4d/0x3e0
> >     [<ffffffff81088667>] kthread+0x117/0x150
> >     [<ffffffff81713fa7>] ret_from_fork+0x27/0x40
> >     [<ffffffffffffffff>] 0xffffffffffffffff
> > 
> > Here is patch to use the older rdma contexts while reposting
> > the isert commands intead of reinitialising them.
> > 
> > Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> > Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> >  drivers/infiniband/ulp/isert/ib_isert.c | 7 +++++++
> >  drivers/infiniband/ulp/isert/ib_isert.h | 1 +
> >  2 files changed, 8 insertions(+)
> >
> 
> So this RFC got Reviewed-by Sagi, is it to be applied now or something
> else??
> 
> If yes, what tree does it want to go through? I see Nicholas and Doug
> have both been historically taking isert patches.

Hi Jason,
Yes, this needs to be applied now, to the target-pending tree.
This fixes the "Queue-full handling" change that is already part of stable trees:
https://www.spinics.net/lists/target-devel/msg13574.html
This also needs to be tagged to stable trees.
As the Queue-full handling changes are already in linux, I think this can go 
through rdma tree aswell.

Thanks,
Bharat
> 
> Thanks,
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Dec. 18, 2017, 11:14 p.m. UTC | #6
On Tue, Nov 28, 2017 at 11:58:07PM +0530, Bharat Potnuri wrote:
> isert commands that failed during isert_rdma_rw_ctx_post() are queued to
> Queue-Full(QF) queue and are scheduled to be reposted during queue-full
> queue processing. During this reposting, the rdma contexts are initialised
> again in isert_rdma_rw_ctx_post(), which is leaking significant memory.
> 
> unreferenced object 0xffff8830201d9640 (size 64):
>   comm "kworker/0:2", pid 195, jiffies 4295374851 (age 4528.436s)
>   hex dump (first 32 bytes):
>     00 60 8b cb 2e 00 00 00 00 10 00 00 00 00 00 00  .`..............
>     00 90 e3 cb 2e 00 00 00 00 10 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff8170711e>] kmemleak_alloc+0x4e/0xb0
>     [<ffffffff811f8ba5>] __kmalloc+0x125/0x2b0
>     [<ffffffffa046b24f>] rdma_rw_ctx_init+0x15f/0x6f0 [ib_core]
>     [<ffffffffa07ab644>] isert_rdma_rw_ctx_post+0xc4/0x3c0 [ib_isert]
>     [<ffffffffa07ad972>] isert_put_datain+0x112/0x1c0 [ib_isert]
>     [<ffffffffa07dddce>] lio_queue_data_in+0x2e/0x30 [iscsi_target_mod]
>     [<ffffffffa076c322>] target_qf_do_work+0x2b2/0x4b0 [target_core_mod]
>     [<ffffffff81080c3b>] process_one_work+0x1db/0x5d0
>     [<ffffffff8108107d>] worker_thread+0x4d/0x3e0
>     [<ffffffff81088667>] kthread+0x117/0x150
>     [<ffffffff81713fa7>] ret_from_fork+0x27/0x40
>     [<ffffffffffffffff>] 0xffffffffffffffff
> 
> Here is patch to use the older rdma contexts while reposting
> the isert commands intead of reinitialising them.
> 
> Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
>  drivers/infiniband/ulp/isert/ib_isert.c | 7 +++++++
>  drivers/infiniband/ulp/isert/ib_isert.h | 1 +
>  2 files changed, 8 insertions(+)

Thanks, applied to RDMA for-next

Jason
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicholas A. Bellinger Jan. 13, 2018, 6:20 a.m. UTC | #7
Hi Jason & Co,

Thanks for picking this up.

On Mon, 2017-12-18 at 16:14 -0700, Jason Gunthorpe wrote:
> On Tue, Nov 28, 2017 at 11:58:07PM +0530, Bharat Potnuri wrote:
> > isert commands that failed during isert_rdma_rw_ctx_post() are queued to
> > Queue-Full(QF) queue and are scheduled to be reposted during queue-full
> > queue processing. During this reposting, the rdma contexts are initialised
> > again in isert_rdma_rw_ctx_post(), which is leaking significant memory.
> > 
> > unreferenced object 0xffff8830201d9640 (size 64):
> >   comm "kworker/0:2", pid 195, jiffies 4295374851 (age 4528.436s)
> >   hex dump (first 32 bytes):
> >     00 60 8b cb 2e 00 00 00 00 10 00 00 00 00 00 00  .`..............
> >     00 90 e3 cb 2e 00 00 00 00 10 00 00 00 00 00 00  ................
> >   backtrace:
> >     [<ffffffff8170711e>] kmemleak_alloc+0x4e/0xb0
> >     [<ffffffff811f8ba5>] __kmalloc+0x125/0x2b0
> >     [<ffffffffa046b24f>] rdma_rw_ctx_init+0x15f/0x6f0 [ib_core]
> >     [<ffffffffa07ab644>] isert_rdma_rw_ctx_post+0xc4/0x3c0 [ib_isert]
> >     [<ffffffffa07ad972>] isert_put_datain+0x112/0x1c0 [ib_isert]
> >     [<ffffffffa07dddce>] lio_queue_data_in+0x2e/0x30 [iscsi_target_mod]
> >     [<ffffffffa076c322>] target_qf_do_work+0x2b2/0x4b0 [target_core_mod]
> >     [<ffffffff81080c3b>] process_one_work+0x1db/0x5d0
> >     [<ffffffff8108107d>] worker_thread+0x4d/0x3e0
> >     [<ffffffff81088667>] kthread+0x117/0x150
> >     [<ffffffff81713fa7>] ret_from_fork+0x27/0x40
> >     [<ffffffffffffffff>] 0xffffffffffffffff
> > 
> > Here is patch to use the older rdma contexts while reposting
> > the isert commands intead of reinitialising them.
> > 
> > Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> > Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> >  drivers/infiniband/ulp/isert/ib_isert.c | 7 +++++++
> >  drivers/infiniband/ulp/isert/ib_isert.h | 1 +
> >  2 files changed, 8 insertions(+)
> 
> Thanks, applied to RDMA for-next
> 
> Jason

Per Potnuri, it should be stable CC' to 4.11.y, and depends on:

7a56dc8 iser-target: avoid posting a recv buffer twice
555a65f iser-target: Fix queue-full response handling
fa7e25c target: Fix unknown fabric callback queue-full errors

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Jan. 13, 2018, 6:04 p.m. UTC | #8
On Fri, Jan 12, 2018 at 10:20:43PM -0800, Nicholas A. Bellinger wrote:

> Per Potnuri, it should be stable CC' to 4.11.y, and depends on:
> 
> 7a56dc8 iser-target: avoid posting a recv buffer twice
> 555a65f iser-target: Fix queue-full response handling
> fa7e25c target: Fix unknown fabric callback queue-full errors

Hi Nicholas,

It is too late for us to revise this commit message in the RDMA tree,
someone will have to notify stable manually instead.

Jason
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sagi Grimberg Jan. 14, 2018, 9:26 a.m. UTC | #9
>> Per Potnuri, it should be stable CC' to 4.11.y, and depends on:
>>
>> 7a56dc8 iser-target: avoid posting a recv buffer twice
>> 555a65f iser-target: Fix queue-full response handling
>> fa7e25c target: Fix unknown fabric callback queue-full errors
> 
> Hi Nicholas,
> 
> It is too late for us to revise this commit message in the RDMA tree,
> someone will have to notify stable manually instead.

I think it would be easier if nab would take this through the target
tree... Nic?
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Jan. 14, 2018, 6:23 p.m. UTC | #10
On Sun, Jan 14, 2018 at 11:26:04AM +0200, Sagi Grimberg wrote:
> 
> >>Per Potnuri, it should be stable CC' to 4.11.y, and depends on:
> >>
> >>7a56dc8 iser-target: avoid posting a recv buffer twice
> >>555a65f iser-target: Fix queue-full response handling
> >>fa7e25c target: Fix unknown fabric callback queue-full errors
> >
> >Hi Nicholas,
> >
> >It is too late for us to revise this commit message in the RDMA tree,
> >someone will have to notify stable manually instead.
> 
> I think it would be easier if nab would take this through the target
> tree... Nic?

Too late for that too, it is in the rdma tree already.

Jason
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicholas A. Bellinger Jan. 15, 2018, 4:39 a.m. UTC | #11
On Sun, 2018-01-14 at 11:23 -0700, Jason Gunthorpe wrote:
> On Sun, Jan 14, 2018 at 11:26:04AM +0200, Sagi Grimberg wrote:
> > 
> > >>Per Potnuri, it should be stable CC' to 4.11.y, and depends on:
> > >>
> > >>7a56dc8 iser-target: avoid posting a recv buffer twice
> > >>555a65f iser-target: Fix queue-full response handling
> > >>fa7e25c target: Fix unknown fabric callback queue-full errors
> > >
> > >Hi Nicholas,
> > >
> > >It is too late for us to revise this commit message in the RDMA tree,
> > >someone will have to notify stable manually instead.
> > 
> > I think it would be easier if nab would take this through the target
> > tree... Nic?
> 
> Too late for that too, it is in the rdma tree already.
> 

No worries.  Will send this out with the next round of stable backports,
post -rc1.

Btw, since this patch + earlier 4.11.y dependencies appear to be a hard
requirement for reliable iser-target operation for RNICs that generate
queue-full retries on large block workloads, I think the dependencies
have been upstream long enough to consider a 4.1.y+ backport as well.
(Adding Bryant CC')

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 720dfb3a1ac2..fd55163801a3 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -2123,6 +2123,9 @@  isert_rdma_rw_ctx_post(struct isert_cmd *cmd, struct isert_conn *conn,
 	u32 rkey, offset;
 	int ret;
 
+	if (cmd->ctx_init_done)
+		goto rdma_ctx_post;
+
 	if (dir == DMA_FROM_DEVICE) {
 		addr = cmd->write_va;
 		rkey = cmd->write_stag;
@@ -2150,11 +2153,15 @@  isert_rdma_rw_ctx_post(struct isert_cmd *cmd, struct isert_conn *conn,
 				se_cmd->t_data_sg, se_cmd->t_data_nents,
 				offset, addr, rkey, dir);
 	}
+
 	if (ret < 0) {
 		isert_err("Cmd: %p failed to prepare RDMA res\n", cmd);
 		return ret;
 	}
 
+	cmd->ctx_init_done = true;
+
+rdma_ctx_post:
 	ret = rdma_rw_ctx_post(&cmd->rw, conn->qp, port_num, cqe, chain_wr);
 	if (ret < 0)
 		isert_err("Cmd: %p failed to post RDMA res\n", cmd);
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index d6fd248320ae..3b296bac4f60 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -126,6 +126,7 @@  struct isert_cmd {
 	struct rdma_rw_ctx	rw;
 	struct work_struct	comp_work;
 	struct scatterlist	sg;
+	bool			ctx_init_done;
 };
 
 static inline struct isert_cmd *tx_desc_to_cmd(struct iser_tx_desc *desc)