| Message ID | 1488219394-11720-1-git-send-email-sagi@grimberg.me (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Very similar to Vladimir's patch. Passed our verification tests. Reviewed-by: Max Gurtovoy <maxg@mellanox.com> Raju, Have you tried this fix in your labs ? On 2/27/2017 8:16 PM, Sagi Grimberg wrote: > When the rdma device is removed, we must cleanup all > the rdma resources within the DEVICE_REMOVAL event > handler to let the device teardown gracefully. When > this happens with live I/O, some memory regions are > occupied. Thus, track them too and dereg all the mr's. > > We are safe with mr access by iscsi_iser_cleanup_task. > > Reported-by: Raju Rangoju <rajur@chelsio.com> > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > drivers/infiniband/ulp/iser/iscsi_iser.h | 2 ++ > drivers/infiniband/ulp/iser/iser_verbs.c | 8 +++++--- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h > index 9d0b22ad58c1..c1ae4aeae2f9 100644 > --- a/drivers/infiniband/ulp/iser/iscsi_iser.h > +++ b/drivers/infiniband/ulp/iser/iscsi_iser.h > @@ -430,6 +430,7 @@ struct iser_fr_desc { > struct list_head list; > struct iser_reg_resources rsc; > struct iser_pi_context *pi_ctx; > + struct list_head all_list; > }; > > /** > @@ -443,6 +444,7 @@ struct iser_fr_pool { > struct list_head list; > spinlock_t lock; > int size; > + struct list_head all_list; > }; > > /** > diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c > index 30b622f2ab73..c538a38c91ce 100644 > --- a/drivers/infiniband/ulp/iser/iser_verbs.c > +++ b/drivers/infiniband/ulp/iser/iser_verbs.c > @@ -362,6 +362,7 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, > int i, ret; > > INIT_LIST_HEAD(&fr_pool->list); > + INIT_LIST_HEAD(&fr_pool->all_list); > spin_lock_init(&fr_pool->lock); > fr_pool->size = 0; > for (i = 0; i < cmds_max; i++) { > @@ -373,6 +374,7 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, > } > > list_add_tail(&desc->list, &fr_pool->list); > + list_add_tail(&desc->all_list, &fr_pool->all_list); > fr_pool->size++; > } > > @@ -392,13 +394,13 @@ void iser_free_fastreg_pool(struct ib_conn *ib_conn) > struct iser_fr_desc *desc, *tmp; > int i = 0; > > - if (list_empty(&fr_pool->list)) > + if (list_empty(&fr_pool->all_list)) > return; > > iser_info("freeing conn %p fr pool\n", ib_conn); > > - list_for_each_entry_safe(desc, tmp, &fr_pool->list, list) { > - list_del(&desc->list); > + list_for_each_entry_safe(desc, tmp, &fr_pool->all_list, all_list) { > + list_del(&desc->all_list); > iser_free_reg_res(&desc->rsc); > if (desc->pi_ctx) > iser_free_pi_ctx(desc->pi_ctx); > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Yes Max, I have tried the fix and it passed our verification tests. -Thanks -----Original Message----- From: Max Gurtovoy [mailto:maxg@mellanox.com] Sent: 28 February 2017 18:58 To: Sagi Grimberg <sagi@grimberg.me>; linux-rdma@vger.kernel.org Cc: vladimirn@mellanox.com; Raju Rangoju <rajur@chelsio.com> Subject: Re: [PATCH] RDMA/iser: Fix possible mr leak on device removal event Very similar to Vladimir's patch. Passed our verification tests. Reviewed-by: Max Gurtovoy <maxg@mellanox.com> Raju, Have you tried this fix in your labs ? On 2/27/2017 8:16 PM, Sagi Grimberg wrote: > When the rdma device is removed, we must cleanup all the rdma > resources within the DEVICE_REMOVAL event handler to let the device > teardown gracefully. When this happens with live I/O, some memory > regions are occupied. Thus, track them too and dereg all the mr's. > > We are safe with mr access by iscsi_iser_cleanup_task. > > Reported-by: Raju Rangoju <rajur@chelsio.com> > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > drivers/infiniband/ulp/iser/iscsi_iser.h | 2 ++ > drivers/infiniband/ulp/iser/iser_verbs.c | 8 +++++--- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h > b/drivers/infiniband/ulp/iser/iscsi_iser.h > index 9d0b22ad58c1..c1ae4aeae2f9 100644 > --- a/drivers/infiniband/ulp/iser/iscsi_iser.h > +++ b/drivers/infiniband/ulp/iser/iscsi_iser.h > @@ -430,6 +430,7 @@ struct iser_fr_desc { > struct list_head list; > struct iser_reg_resources rsc; > struct iser_pi_context *pi_ctx; > + struct list_head all_list; > }; > > /** > @@ -443,6 +444,7 @@ struct iser_fr_pool { > struct list_head list; > spinlock_t lock; > int size; > + struct list_head all_list; > }; > > /** > diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c > b/drivers/infiniband/ulp/iser/iser_verbs.c > index 30b622f2ab73..c538a38c91ce 100644 > --- a/drivers/infiniband/ulp/iser/iser_verbs.c > +++ b/drivers/infiniband/ulp/iser/iser_verbs.c > @@ -362,6 +362,7 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, > int i, ret; > > INIT_LIST_HEAD(&fr_pool->list); > + INIT_LIST_HEAD(&fr_pool->all_list); > spin_lock_init(&fr_pool->lock); > fr_pool->size = 0; > for (i = 0; i < cmds_max; i++) { > @@ -373,6 +374,7 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, > } > > list_add_tail(&desc->list, &fr_pool->list); > + list_add_tail(&desc->all_list, &fr_pool->all_list); > fr_pool->size++; > } > > @@ -392,13 +394,13 @@ void iser_free_fastreg_pool(struct ib_conn *ib_conn) > struct iser_fr_desc *desc, *tmp; > int i = 0; > > - if (list_empty(&fr_pool->list)) > + if (list_empty(&fr_pool->all_list)) > return; > > iser_info("freeing conn %p fr pool\n", ib_conn); > > - list_for_each_entry_safe(desc, tmp, &fr_pool->list, list) { > - list_del(&desc->list); > + list_for_each_entry_safe(desc, tmp, &fr_pool->all_list, all_list) { > + list_del(&desc->all_list); > iser_free_reg_res(&desc->rsc); > if (desc->pi_ctx) > iser_free_pi_ctx(desc->pi_ctx); > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Hi Sagi, Hi Raju, > Can we expect this patch in next rc ? I hope so, Doug, care to get it in the next -rc pull request? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2017-03-13 at 10:06 +0200, Sagi Grimberg wrote: > > > > Hi Sagi, > > Hi Raju, > > > > > Can we expect this patch in next rc ? > > I hope so, > > Doug, care to get it in the next -rc pull request? Done.
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h index 9d0b22ad58c1..c1ae4aeae2f9 100644 --- a/drivers/infiniband/ulp/iser/iscsi_iser.h +++ b/drivers/infiniband/ulp/iser/iscsi_iser.h @@ -430,6 +430,7 @@ struct iser_fr_desc { struct list_head list; struct iser_reg_resources rsc; struct iser_pi_context *pi_ctx; + struct list_head all_list; }; /** @@ -443,6 +444,7 @@ struct iser_fr_pool { struct list_head list; spinlock_t lock; int size; + struct list_head all_list; }; /** diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c index 30b622f2ab73..c538a38c91ce 100644 --- a/drivers/infiniband/ulp/iser/iser_verbs.c +++ b/drivers/infiniband/ulp/iser/iser_verbs.c @@ -362,6 +362,7 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, int i, ret; INIT_LIST_HEAD(&fr_pool->list); + INIT_LIST_HEAD(&fr_pool->all_list); spin_lock_init(&fr_pool->lock); fr_pool->size = 0; for (i = 0; i < cmds_max; i++) { @@ -373,6 +374,7 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, } list_add_tail(&desc->list, &fr_pool->list); + list_add_tail(&desc->all_list, &fr_pool->all_list); fr_pool->size++; } @@ -392,13 +394,13 @@ void iser_free_fastreg_pool(struct ib_conn *ib_conn) struct iser_fr_desc *desc, *tmp; int i = 0; - if (list_empty(&fr_pool->list)) + if (list_empty(&fr_pool->all_list)) return; iser_info("freeing conn %p fr pool\n", ib_conn); - list_for_each_entry_safe(desc, tmp, &fr_pool->list, list) { - list_del(&desc->list); + list_for_each_entry_safe(desc, tmp, &fr_pool->all_list, all_list) { + list_del(&desc->all_list); iser_free_reg_res(&desc->rsc); if (desc->pi_ctx) iser_free_pi_ctx(desc->pi_ctx);
When the rdma device is removed, we must cleanup all the rdma resources within the DEVICE_REMOVAL event handler to let the device teardown gracefully. When this happens with live I/O, some memory regions are occupied. Thus, track them too and dereg all the mr's. We are safe with mr access by iscsi_iser_cleanup_task. Reported-by: Raju Rangoju <rajur@chelsio.com> Signed-off-by: Sagi Grimberg <sagi@grimberg.me> --- drivers/infiniband/ulp/iser/iscsi_iser.h | 2 ++ drivers/infiniband/ulp/iser/iser_verbs.c | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-)