linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] IB/iSER-Target: Release connection resources properly when receiving RDMA_CM_EVENT_DEVICE_REMOVAL
@ 2016-07-27  5:09 Raju Rangoju
       [not found] ` <20160727050918.12772-1-rajur-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Raju Rangoju @ 2016-07-27  5:09 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
	sagi-NQWnxTmZq1alnMjI0IkVqw, Raju Rangoju

When the low level driver exercises the hot unplug they would call
rdma_cm cma_remove_one which would fire DEVICE_REMOVAL event to all cma
consumers. Now, if consumer doesn't make sure they destroy all IB
objects created on that IB device instance prior to finalizing all
processing of DEVICE_REMOVAL callback, rdma_cm will let the lld to
de-register with IB core and destroy the IB device instance. And if the
consumer calls (say) ib_dereg_mr(), it will crash since that dev object
is NULL.

In the current implementation, iser-target just initiates the cleanup
and returns from DEVICE_REMOVAL callback. This deferred work creates a
race between iser-target cleaning IB objects(say MR) and lld destroying
IB device instance.

This patch includes the following fixes
  -> make sure that consumer frees all IB objects associated with device
     instance
  -> return non-zero from the callback to destroy the rdma_cm id
---
 drivers/infiniband/ulp/isert/ib_isert.c | 24 ++++++++++++++++++++++--
 drivers/infiniband/ulp/isert/ib_isert.h |  2 ++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index a990c04..9adc38d 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -405,6 +405,7 @@ isert_init_conn(struct isert_conn *isert_conn)
 	INIT_LIST_HEAD(&isert_conn->node);
 	init_completion(&isert_conn->login_comp);
 	init_completion(&isert_conn->login_req_comp);
+	init_waitqueue_head(&isert_conn->rem_wait);
 	kref_init(&isert_conn->kref);
 	mutex_init(&isert_conn->mutex);
 	INIT_WORK(&isert_conn->release_work, isert_release_work);
@@ -580,7 +581,8 @@ isert_connect_release(struct isert_conn *isert_conn)
 	BUG_ON(!device);
 
 	isert_free_rx_descriptors(isert_conn);
-	if (isert_conn->cm_id)
+	if (isert_conn->cm_id &&
+	    !isert_conn->dev_removed)
 		rdma_destroy_id(isert_conn->cm_id);
 
 	if (isert_conn->qp) {
@@ -595,7 +597,10 @@ isert_connect_release(struct isert_conn *isert_conn)
 
 	isert_device_put(device);
 
-	kfree(isert_conn);
+	if (isert_conn->dev_removed)
+		wake_up_interruptible(&isert_conn->rem_wait);
+	else
+		kfree(isert_conn);
 }
 
 static void
@@ -755,6 +760,7 @@ static int
 isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 {
 	struct isert_np *isert_np = cma_id->context;
+	struct isert_conn *isert_conn;
 	int ret = 0;
 
 	isert_info("%s (%d): status %d id %p np %p\n",
@@ -778,6 +784,20 @@ isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 	case RDMA_CM_EVENT_DEVICE_REMOVAL: /* FALLTHRU */
 	case RDMA_CM_EVENT_TIMEWAIT_EXIT:  /* FALLTHRU */
 		ret = isert_disconnected_handler(cma_id, event->event);
+
+		if (event->event == RDMA_CM_EVENT_DEVICE_REMOVAL) {
+			isert_conn = cma_id->qp->qp_context;
+			isert_conn->dev_removed = true;
+			wait_event_interruptible(isert_conn->rem_wait,
+					 isert_conn->state == ISER_CONN_DOWN);
+
+			kfree(isert_conn);
+			 /*
+			  * return non-zero from the callback to destroy
+			  * the rdma cm id
+			  */
+			return 1;
+		}
 		break;
 	case RDMA_CM_EVENT_REJECTED:       /* FALLTHRU */
 	case RDMA_CM_EVENT_UNREACHABLE:    /* FALLTHRU */
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index e512ba9..d0c5c2c 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -159,6 +159,8 @@ struct isert_conn {
 	struct work_struct	release_work;
 	bool                    logout_posted;
 	bool                    snd_w_inv;
+	wait_queue_head_t       rem_wait;
+	bool                    dev_removed;
 };
 
 #define ISERT_MAX_CQ 64
-- 
2.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/1] IB/iSER-Target: Release connection resources properly when receiving RDMA_CM_EVENT_DEVICE_REMOVAL
       [not found] ` <20160727050918.12772-1-rajur-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
@ 2016-07-27 16:28   ` Leon Romanovsky
  0 siblings, 0 replies; 7+ messages in thread
From: Leon Romanovsky @ 2016-07-27 16:28 UTC (permalink / raw)
  To: Raju Rangoju
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
	sagi-NQWnxTmZq1alnMjI0IkVqw

[-- Attachment #1: Type: text/plain, Size: 4422 bytes --]

On Wed, Jul 27, 2016 at 10:39:18AM +0530, Raju Rangoju wrote:
> When the low level driver exercises the hot unplug they would call
> rdma_cm cma_remove_one which would fire DEVICE_REMOVAL event to all cma
> consumers. Now, if consumer doesn't make sure they destroy all IB
> objects created on that IB device instance prior to finalizing all
> processing of DEVICE_REMOVAL callback, rdma_cm will let the lld to
> de-register with IB core and destroy the IB device instance. And if the
> consumer calls (say) ib_dereg_mr(), it will crash since that dev object
> is NULL.
> 
> In the current implementation, iser-target just initiates the cleanup
> and returns from DEVICE_REMOVAL callback. This deferred work creates a
> race between iser-target cleaning IB objects(say MR) and lld destroying
> IB device instance.
> 
> This patch includes the following fixes
>   -> make sure that consumer frees all IB objects associated with device
>      instance
>   -> return non-zero from the callback to destroy the rdma_cm id
> ---
>  drivers/infiniband/ulp/isert/ib_isert.c | 24 ++++++++++++++++++++++--
>  drivers/infiniband/ulp/isert/ib_isert.h |  2 ++
>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> index a990c04..9adc38d 100644
> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> @@ -405,6 +405,7 @@ isert_init_conn(struct isert_conn *isert_conn)
>  	INIT_LIST_HEAD(&isert_conn->node);
>  	init_completion(&isert_conn->login_comp);
>  	init_completion(&isert_conn->login_req_comp);
> +	init_waitqueue_head(&isert_conn->rem_wait);
>  	kref_init(&isert_conn->kref);
>  	mutex_init(&isert_conn->mutex);
>  	INIT_WORK(&isert_conn->release_work, isert_release_work);
> @@ -580,7 +581,8 @@ isert_connect_release(struct isert_conn *isert_conn)
>  	BUG_ON(!device);
>  
>  	isert_free_rx_descriptors(isert_conn);
> -	if (isert_conn->cm_id)
> +	if (isert_conn->cm_id &&
> +	    !isert_conn->dev_removed)
>  		rdma_destroy_id(isert_conn->cm_id);
>  
>  	if (isert_conn->qp) {
> @@ -595,7 +597,10 @@ isert_connect_release(struct isert_conn *isert_conn)
>  
>  	isert_device_put(device);
>  
> -	kfree(isert_conn);
> +	if (isert_conn->dev_removed)
> +		wake_up_interruptible(&isert_conn->rem_wait);
> +	else
> +		kfree(isert_conn);
>  }
>  
>  static void
> @@ -755,6 +760,7 @@ static int
>  isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
>  {
>  	struct isert_np *isert_np = cma_id->context;
> +	struct isert_conn *isert_conn;
>  	int ret = 0;
>  
>  	isert_info("%s (%d): status %d id %p np %p\n",
> @@ -778,6 +784,20 @@ isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
>  	case RDMA_CM_EVENT_DEVICE_REMOVAL: /* FALLTHRU */
>  	case RDMA_CM_EVENT_TIMEWAIT_EXIT:  /* FALLTHRU */
>  		ret = isert_disconnected_handler(cma_id, event->event);
> +
> +		if (event->event == RDMA_CM_EVENT_DEVICE_REMOVAL) {

It will be nicer if you can reshuffle cases in original switch in such
was that will eliminate the need of this "if".

> +			isert_conn = cma_id->qp->qp_context;
> +			isert_conn->dev_removed = true;
> +			wait_event_interruptible(isert_conn->rem_wait,
> +					 isert_conn->state == ISER_CONN_DOWN);
> +
> +			kfree(isert_conn);
> +			 /*
> +			  * return non-zero from the callback to destroy
> +			  * the rdma cm id
> +			  */
> +			return 1;
> +		}
>  		break;
>  	case RDMA_CM_EVENT_REJECTED:       /* FALLTHRU */
>  	case RDMA_CM_EVENT_UNREACHABLE:    /* FALLTHRU */
> diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
> index e512ba9..d0c5c2c 100644
> --- a/drivers/infiniband/ulp/isert/ib_isert.h
> +++ b/drivers/infiniband/ulp/isert/ib_isert.h
> @@ -159,6 +159,8 @@ struct isert_conn {
>  	struct work_struct	release_work;
>  	bool                    logout_posted;
>  	bool                    snd_w_inv;
> +	wait_queue_head_t       rem_wait;
> +	bool                    dev_removed;
>  };
>  
>  #define ISERT_MAX_CQ 64
> -- 
> 2.8.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/1] IB/iSER-Target: Release connection resources properly when receiving RDMA_CM_EVENT_DEVICE_REMOVAL
@ 2016-07-27 19:15 Raju Rangoju
       [not found] ` <20160727191511.18122-1-rajur-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Raju Rangoju @ 2016-07-27 19:15 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
	sagi-NQWnxTmZq1alnMjI0IkVqw, Raju Rangoju

When the low level driver exercises the hot unplug they would call
rdma_cm cma_remove_one which would fire DEVICE_REMOVAL event to all cma
consumers. Now, if consumer doesn't make sure they destroy all IB
objects created on that IB device instance prior to finalizing all
processing of DEVICE_REMOVAL callback, rdma_cm will let the lld to
de-register with IB core and destroy the IB device instance. And if the
consumer calls (say) ib_dereg_mr(), it will crash since that dev object
is NULL.

In the current implementation, iser-target just initiates the cleanup
and returns from DEVICE_REMOVAL callback. This deferred work creates a
race between iser-target cleaning IB objects(say MR) and lld destroying
IB device instance.

This patch includes the following fixes
  -> make sure that consumer frees all IB objects associated with device
     instance
  -> return non-zero from the callback to destroy the rdma_cm id
---
 drivers/infiniband/ulp/isert/ib_isert.c | 23 ++++++++++++++++++++---
 drivers/infiniband/ulp/isert/ib_isert.h |  2 ++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index a990c04..3dfd903 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -405,6 +405,7 @@ isert_init_conn(struct isert_conn *isert_conn)
 	INIT_LIST_HEAD(&isert_conn->node);
 	init_completion(&isert_conn->login_comp);
 	init_completion(&isert_conn->login_req_comp);
+	init_waitqueue_head(&isert_conn->rem_wait);
 	kref_init(&isert_conn->kref);
 	mutex_init(&isert_conn->mutex);
 	INIT_WORK(&isert_conn->release_work, isert_release_work);
@@ -580,7 +581,8 @@ isert_connect_release(struct isert_conn *isert_conn)
 	BUG_ON(!device);
 
 	isert_free_rx_descriptors(isert_conn);
-	if (isert_conn->cm_id)
+	if (isert_conn->cm_id &&
+	    !isert_conn->dev_removed)
 		rdma_destroy_id(isert_conn->cm_id);
 
 	if (isert_conn->qp) {
@@ -595,7 +597,10 @@ isert_connect_release(struct isert_conn *isert_conn)
 
 	isert_device_put(device);
 
-	kfree(isert_conn);
+	if (isert_conn->dev_removed)
+		wake_up_interruptible(&isert_conn->rem_wait);
+	else
+		kfree(isert_conn);
 }
 
 static void
@@ -755,6 +760,7 @@ static int
 isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 {
 	struct isert_np *isert_np = cma_id->context;
+	struct isert_conn *isert_conn;
 	int ret = 0;
 
 	isert_info("%s (%d): status %d id %p np %p\n",
@@ -775,10 +781,21 @@ isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 		break;
 	case RDMA_CM_EVENT_ADDR_CHANGE:    /* FALLTHRU */
 	case RDMA_CM_EVENT_DISCONNECTED:   /* FALLTHRU */
-	case RDMA_CM_EVENT_DEVICE_REMOVAL: /* FALLTHRU */
 	case RDMA_CM_EVENT_TIMEWAIT_EXIT:  /* FALLTHRU */
 		ret = isert_disconnected_handler(cma_id, event->event);
 		break;
+	case RDMA_CM_EVENT_DEVICE_REMOVAL:
+		isert_conn = cma_id->qp->qp_context;
+		isert_conn->dev_removed = true;
+		isert_disconnected_handler(cma_id, event->event);
+		wait_event_interruptible(isert_conn->rem_wait,
+					 isert_conn->state == ISER_CONN_DOWN);
+		kfree(isert_conn);
+		/*
+		 * return non-zero from the callback to destroy
+		 * the rdma cm id
+		 */
+		return 1;
 	case RDMA_CM_EVENT_REJECTED:       /* FALLTHRU */
 	case RDMA_CM_EVENT_UNREACHABLE:    /* FALLTHRU */
 	case RDMA_CM_EVENT_CONNECT_ERROR:
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index e512ba9..d0c5c2c 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -159,6 +159,8 @@ struct isert_conn {
 	struct work_struct	release_work;
 	bool                    logout_posted;
 	bool                    snd_w_inv;
+	wait_queue_head_t       rem_wait;
+	bool                    dev_removed;
 };
 
 #define ISERT_MAX_CQ 64
-- 
2.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/1] IB/iSER-Target: Release connection resources properly when receiving RDMA_CM_EVENT_DEVICE_REMOVAL
       [not found] ` <20160727191511.18122-1-rajur-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
@ 2016-07-29 20:33   ` Sagi Grimberg
       [not found]     ` <22568e6b-e764-bdd4-eec9-dc53a258b371-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
  2016-08-02 17:49   ` Doug Ledford
  1 sibling, 1 reply; 7+ messages in thread
From: Sagi Grimberg @ 2016-07-29 20:33 UTC (permalink / raw)
  To: Raju Rangoju, linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW


> When the low level driver exercises the hot unplug they would call
> rdma_cm cma_remove_one which would fire DEVICE_REMOVAL event to all cma
> consumers. Now, if consumer doesn't make sure they destroy all IB
> objects created on that IB device instance prior to finalizing all
> processing of DEVICE_REMOVAL callback, rdma_cm will let the lld to
> de-register with IB core and destroy the IB device instance. And if the
> consumer calls (say) ib_dereg_mr(), it will crash since that dev object
> is NULL.

Yea... this used to work but sort of broke somewhere...

Thanks Raju, the patch looks good,

Acked-by: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>

Doug,

Can you add a stable tag to this when picking it up?

Thanks,
Sagi
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/1] IB/iSER-Target: Release connection resources properly when receiving RDMA_CM_EVENT_DEVICE_REMOVAL
@ 2016-08-02 10:57 Raju Rangoju
  0 siblings, 0 replies; 7+ messages in thread
From: Raju Rangoju @ 2016-08-02 10:57 UTC (permalink / raw)
  To: target-devel, nab; +Cc: linux-rdma, sagi, swise, Raju Rangoju

When the low level driver exercises the hot unplug they would call
rdma_cm cma_remove_one which would fire DEVICE_REMOVAL event to all cma
consumers. Now, if consumer doesn't make sure they destroy all IB
objects created on that IB device instance prior to finalizing all
processing of DEVICE_REMOVAL callback, rdma_cm will let the lld to
de-register with IB core and destroy the IB device instance. And if the
consumer calls (say) ib_dereg_mr(), it will crash since that dev object
is NULL.

In the current implementation, iser-target just initiates the cleanup
and returns from DEVICE_REMOVAL callback. This deferred work creates a
race between iser-target cleaning IB objects(say MR) and lld destroying
IB device instance.

This patch includes the following fixes
  -> make sure that consumer frees all IB objects associated with device
     instance
  -> return non-zero from the callback to destroy the rdma_cm id

Signed-off-by: Raju Rangoju <rajur@chelsio.com>
Acked-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/infiniband/ulp/isert/ib_isert.c | 23 ++++++++++++++++++++---
 drivers/infiniband/ulp/isert/ib_isert.h |  2 ++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index a990c04..3dfd903 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -405,6 +405,7 @@ isert_init_conn(struct isert_conn *isert_conn)
 	INIT_LIST_HEAD(&isert_conn->node);
 	init_completion(&isert_conn->login_comp);
 	init_completion(&isert_conn->login_req_comp);
+	init_waitqueue_head(&isert_conn->rem_wait);
 	kref_init(&isert_conn->kref);
 	mutex_init(&isert_conn->mutex);
 	INIT_WORK(&isert_conn->release_work, isert_release_work);
@@ -580,7 +581,8 @@ isert_connect_release(struct isert_conn *isert_conn)
 	BUG_ON(!device);
 
 	isert_free_rx_descriptors(isert_conn);
-	if (isert_conn->cm_id)
+	if (isert_conn->cm_id &&
+	    !isert_conn->dev_removed)
 		rdma_destroy_id(isert_conn->cm_id);
 
 	if (isert_conn->qp) {
@@ -595,7 +597,10 @@ isert_connect_release(struct isert_conn *isert_conn)
 
 	isert_device_put(device);
 
-	kfree(isert_conn);
+	if (isert_conn->dev_removed)
+		wake_up_interruptible(&isert_conn->rem_wait);
+	else
+		kfree(isert_conn);
 }
 
 static void
@@ -755,6 +760,7 @@ static int
 isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 {
 	struct isert_np *isert_np = cma_id->context;
+	struct isert_conn *isert_conn;
 	int ret = 0;
 
 	isert_info("%s (%d): status %d id %p np %p\n",
@@ -775,10 +781,21 @@ isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 		break;
 	case RDMA_CM_EVENT_ADDR_CHANGE:    /* FALLTHRU */
 	case RDMA_CM_EVENT_DISCONNECTED:   /* FALLTHRU */
-	case RDMA_CM_EVENT_DEVICE_REMOVAL: /* FALLTHRU */
 	case RDMA_CM_EVENT_TIMEWAIT_EXIT:  /* FALLTHRU */
 		ret = isert_disconnected_handler(cma_id, event->event);
 		break;
+	case RDMA_CM_EVENT_DEVICE_REMOVAL:
+		isert_conn = cma_id->qp->qp_context;
+		isert_conn->dev_removed = true;
+		isert_disconnected_handler(cma_id, event->event);
+		wait_event_interruptible(isert_conn->rem_wait,
+					 isert_conn->state == ISER_CONN_DOWN);
+		kfree(isert_conn);
+		/*
+		 * return non-zero from the callback to destroy
+		 * the rdma cm id
+		 */
+		return 1;
 	case RDMA_CM_EVENT_REJECTED:       /* FALLTHRU */
 	case RDMA_CM_EVENT_UNREACHABLE:    /* FALLTHRU */
 	case RDMA_CM_EVENT_CONNECT_ERROR:
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index e512ba9..d0c5c2c 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -159,6 +159,8 @@ struct isert_conn {
 	struct work_struct	release_work;
 	bool                    logout_posted;
 	bool                    snd_w_inv;
+	wait_queue_head_t       rem_wait;
+	bool                    dev_removed;
 };
 
 #define ISERT_MAX_CQ 64
-- 
2.8.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/1] IB/iSER-Target: Release connection resources properly when receiving RDMA_CM_EVENT_DEVICE_REMOVAL
       [not found] ` <20160727191511.18122-1-rajur-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
  2016-07-29 20:33   ` Sagi Grimberg
@ 2016-08-02 17:49   ` Doug Ledford
  1 sibling, 0 replies; 7+ messages in thread
From: Doug Ledford @ 2016-08-02 17:49 UTC (permalink / raw)
  To: Raju Rangoju, linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
	sagi-NQWnxTmZq1alnMjI0IkVqw

[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]

On Thu, 2016-07-28 at 00:45 +0530, Raju Rangoju wrote:
> When the low level driver exercises the hot unplug they would call
> rdma_cm cma_remove_one which would fire DEVICE_REMOVAL event to all
> cma
> consumers. Now, if consumer doesn't make sure they destroy all IB
> objects created on that IB device instance prior to finalizing all
> processing of DEVICE_REMOVAL callback, rdma_cm will let the lld to
> de-register with IB core and destroy the IB device instance. And if
> the
> consumer calls (say) ib_dereg_mr(), it will crash since that dev
> object
> is NULL.
> 
> In the current implementation, iser-target just initiates the cleanup
> and returns from DEVICE_REMOVAL callback. This deferred work creates
> a
> race between iser-target cleaning IB objects(say MR) and lld
> destroying
> IB device instance.
> 
> This patch includes the following fixes
>   -> make sure that consumer frees all IB objects associated with
> device
>      instance
>   -> return non-zero from the callback to destroy the rdma_cm id

This patch is missing a Signed-off-by: line and can not be accepted as
it is.  Please resubmit with the proper attribution.  Also please
reword your commit subject as it's too long.  I suggest something like:

IB/isert: Properly release resources on RDMA_CM_EVENT_DEVICE_REMOVAL

which is still too long, but not as bad as what you have now.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/1] IB/iSER-Target: Release connection resources properly when receiving RDMA_CM_EVENT_DEVICE_REMOVAL
       [not found]     ` <22568e6b-e764-bdd4-eec9-dc53a258b371-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
@ 2016-08-02 17:50       ` Doug Ledford
  0 siblings, 0 replies; 7+ messages in thread
From: Doug Ledford @ 2016-08-02 17:50 UTC (permalink / raw)
  To: Sagi Grimberg, Raju Rangoju, linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW

[-- Attachment #1: Type: text/plain, Size: 1079 bytes --]

On Fri, 2016-07-29 at 23:33 +0300, Sagi Grimberg wrote:
> > 
> > When the low level driver exercises the hot unplug they would call
> > rdma_cm cma_remove_one which would fire DEVICE_REMOVAL event to all
> > cma
> > consumers. Now, if consumer doesn't make sure they destroy all IB
> > objects created on that IB device instance prior to finalizing all
> > processing of DEVICE_REMOVAL callback, rdma_cm will let the lld to
> > de-register with IB core and destroy the IB device instance. And if
> > the
> > consumer calls (say) ib_dereg_mr(), it will crash since that dev
> > object
> > is NULL.
> 
> Yea... this used to work but sort of broke somewhere...
> 
> Thanks Raju, the patch looks good,
> 
> Acked-by: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
> 
> Doug,
> 
> Can you add a stable tag to this when picking it up?
> 

I can add a stable tag, but it helps to know what versions of stable it
is expected to apply to.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-08-02 17:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-27 19:15 [PATCH 1/1] IB/iSER-Target: Release connection resources properly when receiving RDMA_CM_EVENT_DEVICE_REMOVAL Raju Rangoju
     [not found] ` <20160727191511.18122-1-rajur-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
2016-07-29 20:33   ` Sagi Grimberg
     [not found]     ` <22568e6b-e764-bdd4-eec9-dc53a258b371-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
2016-08-02 17:50       ` Doug Ledford
2016-08-02 17:49   ` Doug Ledford
  -- strict thread matches above, loose matches on Subject: below --
2016-08-02 10:57 Raju Rangoju
2016-07-27  5:09 Raju Rangoju
     [not found] ` <20160727050918.12772-1-rajur-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
2016-07-27 16:28   ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).