[RESEND PATCH] BNX2I - Bug fixes related to MTU change issue when there are active iscsi sessions

[RESEND PATCH] BNX2I - Bug fixes related to MTU change issue when there are active iscsi sessions

[Anil Veerabhadrappa]

James,
   Please apply this patch to fix resources leak issue when device MTU
is changed or ethtool selftest is executed while there are active iscsi
sessions.

Thanks,
Anil


    * bnx2i driver has to wait and cleanup all iscsi endpoints before
      returning from bnx2i_stop(). This is to make sure all chip
      resources are freed before chip is reset.
    * As the requirements for 1G and 10G chipsets is different, added
      per-device 'hba_shutdown_tmo' parameter to adapter structure
    * If the connections are not torn down by the daemon within this
      timeout period, 'cid's will be leaked in 10G device. 1G devices
      are more flexible and do not leak any resources because the whole
      chip ports gets reset when MTU is changed or ethtool selftest is run
    * fixed a minor issue in bnx2i_ep_poll() which unnecessarily forced
      error return code when driver timed out waiting for TCP connect
      request to complete

Signed-off-by: Anil Veerabhadrappa <anilgv@broadcom.com>
---
 drivers/scsi/bnx2i/bnx2i.h       |    2 ++
 drivers/scsi/bnx2i/bnx2i_init.c  |   13 ++++++++++++-
 drivers/scsi/bnx2i/bnx2i_iscsi.c |   13 ++++++++++---
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/bnx2i/bnx2i.h b/drivers/scsi/bnx2i/bnx2i.h
index 6cf9dc3..6b624e7 100644
--- a/drivers/scsi/bnx2i/bnx2i.h
+++ b/drivers/scsi/bnx2i/bnx2i.h
@@ -362,6 +362,7 @@ struct bnx2i_hba {
 	u32 num_ccell;
 
 	int ofld_conns_active;
+	wait_queue_head_t eh_wait;
 
 	int max_active_conns;
 	struct iscsi_cid_queue cid_que;
@@ -381,6 +382,7 @@ struct bnx2i_hba {
 	spinlock_t lock;	/* protects hba structure access */
 	struct mutex net_dev_lock;/* sync net device access */
 
+	int hba_shutdown_tmo;
 	/*
 	 * PCI related info.
 	 */
diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c
index 6d8172e..5d9296c 100644
--- a/drivers/scsi/bnx2i/bnx2i_init.c
+++ b/drivers/scsi/bnx2i/bnx2i_init.c
@@ -177,11 +177,22 @@ void bnx2i_stop(void *handle)
 	struct bnx2i_hba *hba = handle;
 
 	/* check if cleanup happened in GOING_DOWN context */
-	clear_bit(ADAPTER_STATE_UP, &hba->adapter_state);
 	if (!test_and_clear_bit(ADAPTER_STATE_GOING_DOWN,
 				&hba->adapter_state))
 		iscsi_host_for_each_session(hba->shost,
 					    bnx2i_drop_session);
+
+	/* Wait for all endpoints to be torn down, Chip will be reset once
+	 *  control returns to network driver. So it is required to cleanup and
+	 * release all connection resources before returning from this routine.
+	 */
+	wait_event_interruptible_timeout(hba->eh_wait,
+					 (hba->ofld_conns_active == 0),
+					 hba->hba_shutdown_tmo);
+	/* This flag should be cleared last so that ep_disconnect() gracefully
+	 * cleans up connection context
+	 */
+	clear_bit(ADAPTER_STATE_UP, &hba->adapter_state);
 }
 
 /**
diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index cb71dc9..639e7d8 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -819,6 +819,11 @@ struct bnx2i_hba *bnx2i_alloc_hba(struct cnic_dev *cnic)
 
 	spin_lock_init(&hba->lock);
 	mutex_init(&hba->net_dev_lock);
+	init_waitqueue_head(&hba->eh_wait);
+	if (test_bit(BNX2I_NX2_DEV_57710, &hba->cnic_dev_type))
+		hba->hba_shutdown_tmo = 240 * HZ;
+	else	/* 5706/5708/5709 */
+		hba->hba_shutdown_tmo = 30 * HZ;
 
 	if (iscsi_host_add(shost, &hba->pcidev->dev))
 		goto free_dump_mem;
@@ -1657,8 +1662,8 @@ static struct iscsi_endpoint *bnx2i_ep_connect(struct Scsi_Host *shost,
 		 */
 		hba = bnx2i_check_route(dst_addr);
 
-	if (!hba) {
-		rc = -ENOMEM;
+	if (!hba || test_bit(ADAPTER_STATE_GOING_DOWN, &hba->adapter_state)) {
+		rc = -EINVAL;
 		goto check_busy;
 	}
 
@@ -1803,7 +1808,7 @@ static int bnx2i_ep_poll(struct iscsi_endpoint *ep, int timeout_ms)
 					       (bnx2i_ep->state ==
 						EP_STATE_CONNECT_COMPL)),
 					      msecs_to_jiffies(timeout_ms));
-	if (!rc || (bnx2i_ep->state == EP_STATE_OFLD_FAILED))
+	if (bnx2i_ep->state == EP_STATE_OFLD_FAILED)
 		rc = -1;
 
 	if (rc > 0)
@@ -1956,6 +1961,8 @@ return_bnx2i_ep:
 
 	if (!hba->ofld_conns_active)
 		bnx2i_unreg_dev_all();
+
+	wake_up_interruptible(&hba->eh_wait);
 }
 
 
-- 
1.6.5.1

Re: [RESEND PATCH] BNX2I - Bug fixes related to MTU change issue when there are active iscsi sessions

[Mike Christie]

On 04/08/2010 05:59 PM, Anil Veerabhadrappa wrote:
> James,
>     Please apply this patch to fix resources leak issue when device MTU
> is changed or ethtool selftest is executed while there are active iscsi
> sessions.
>
> Thanks,
> Anil
>
>
>      * bnx2i driver has to wait and cleanup all iscsi endpoints before
>        returning from bnx2i_stop(). This is to make sure all chip
>        resources are freed before chip is reset.
>      * As the requirements for 1G and 10G chipsets is different, added
>        per-device 'hba_shutdown_tmo' parameter to adapter structure
>      * If the connections are not torn down by the daemon within this
>        timeout period, 'cid's will be leaked in 10G device. 1G devices
>        are more flexible and do not leak any resources because the whole
>        chip ports gets reset when MTU is changed or ethtool selftest is run
>      * fixed a minor issue in bnx2i_ep_poll() which unnecessarily forced
>        error return code when driver timed out waiting for TCP connect
>        request to complete
>
> Signed-off-by: Anil Veerabhadrappa<anilgv@broadcom.com>


Looks ok now.

Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>