]> bbs.cooldavid.org Git - net-next-2.6.git/commitdiff
[SCSI] be2iscsi: fix disconnection cleanup
authorMike Christie <michaelc@cs.wisc.edu>
Wed, 9 Jun 2010 08:30:08 +0000 (03:30 -0500)
committerJames Bottomley <James.Bottomley@suse.de>
Tue, 27 Jul 2010 17:01:39 +0000 (12:01 -0500)
This patch fixes 4 bugs in the connection connect/disconnect
cleanup path.

1. If beiscsi_open_conn fails beiscsi_free_ep was always being
called, and if beiscsi_open_conn failed because beiscsi_get_cid
failed then we would free an unallocated cid.

2. If beiscsi_ep_connect failed due to a beiscsi_open_conn failure
it was leaking iscsi_endpoints.

3. beiscsi_ep_disconnect was leaking iscsi_endpoints.
beiscsi_ep_disconnect should free the iscsi_endpoint. We cannot
do it in beiscsi_conn_stop because that is only called for
iscsi connection cleanup. If beiscsi_ep_connect returns
success, but then the poll function fails or the connect
times out then beiscsi_ep_disconnect will be called to clean
up the ep. The conn_stop callout will not be called in that path.

4. beiscsi_conn_stop was freeing the iscsi_endpoint then accessing
it a couple lines later.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
Signed-off-by: James Bottomley <James.Bottomley@suse.de>
drivers/scsi/be2iscsi/be_iscsi.c
drivers/scsi/be2iscsi/be_iscsi.h
drivers/scsi/be2iscsi/be_main.c

index c3928cb8b04208c5dcddd915014c24e2ac3e4b51..454027ccbf16fa47d004e55a9ef949f55e53b827 100644 (file)
@@ -441,6 +441,31 @@ static int beiscsi_get_cid(struct beiscsi_hba *phba)
        return cid;
 }
 
+/**
+ * beiscsi_put_cid - Free the cid
+ * @phba: The phba for which the cid is being freed
+ * @cid: The cid to free
+ */
+static void beiscsi_put_cid(struct beiscsi_hba *phba, unsigned short cid)
+{
+       phba->avlbl_cids++;
+       phba->cid_array[phba->cid_free++] = cid;
+       if (phba->cid_free == phba->params.cxns_per_ctrl)
+               phba->cid_free = 0;
+}
+
+/**
+ * beiscsi_free_ep - free endpoint
+ * @ep:        pointer to iscsi endpoint structure
+ */
+static void beiscsi_free_ep(struct beiscsi_endpoint *beiscsi_ep)
+{
+       struct beiscsi_hba *phba = beiscsi_ep->phba;
+
+       beiscsi_put_cid(phba, beiscsi_ep->ep_cid);
+       beiscsi_ep->phba = NULL;
+}
+
 /**
  * beiscsi_open_conn - Ask FW to open a TCP connection
  * @ep:        endpoint to be used
@@ -475,7 +500,7 @@ static int beiscsi_open_conn(struct iscsi_endpoint *ep,
        if (beiscsi_ep->ep_cid > (phba->fw_config.iscsi_cid_start +
                                  phba->params.cxns_per_ctrl * 2)) {
                SE_DEBUG(DBG_LVL_1, "Failed in allocate iscsi cid\n");
-               return ret;
+               goto free_ep;
        }
 
        beiscsi_ep->cid_vld = 0;
@@ -493,10 +518,10 @@ static int beiscsi_open_conn(struct iscsi_endpoint *ep,
        status = phba->ctrl.mcc_numtag[tag] & 0x000000FF;
        if (status || extd_status) {
                SE_DEBUG(DBG_LVL_1, "mgmt_open_connection Failed"
-                                   " status = %d extd_status = %d \n",
+                                   " status = %d extd_status = %d\n",
                                    status, extd_status);
                free_mcc_tag(&phba->ctrl, tag);
-               return -1;
+               goto free_ep;
        } else {
                wrb = queue_get_wrb(mccq, wrb_num);
                free_mcc_tag(&phba->ctrl, tag);
@@ -508,31 +533,10 @@ static int beiscsi_open_conn(struct iscsi_endpoint *ep,
                SE_DEBUG(DBG_LVL_8, "mgmt_open_connection Success\n");
        }
        return 0;
-}
-
-/**
- * beiscsi_put_cid - Free the cid
- * @phba: The phba for which the cid is being freed
- * @cid: The cid to free
- */
-static void beiscsi_put_cid(struct beiscsi_hba *phba, unsigned short cid)
-{
-       phba->avlbl_cids++;
-       phba->cid_array[phba->cid_free++] = cid;
-       if (phba->cid_free == phba->params.cxns_per_ctrl)
-               phba->cid_free = 0;
-}
-
-/**
- * beiscsi_free_ep - free endpoint
- * @ep:        pointer to iscsi endpoint structure
- */
-static void beiscsi_free_ep(struct beiscsi_endpoint *beiscsi_ep)
-{
-       struct beiscsi_hba *phba = beiscsi_ep->phba;
 
-       beiscsi_put_cid(phba, beiscsi_ep->ep_cid);
-       beiscsi_ep->phba = NULL;
+free_ep:
+       beiscsi_free_ep(beiscsi_ep);
+       return -1;
 }
 
 /**
@@ -585,7 +589,7 @@ beiscsi_ep_connect(struct Scsi_Host *shost, struct sockaddr *dst_addr,
        return ep;
 
 free_ep:
-       beiscsi_free_ep(beiscsi_ep);
+       iscsi_destroy_endpoint(ep);
        return ERR_PTR(ret);
 }
 
@@ -631,30 +635,6 @@ static int beiscsi_close_conn(struct  beiscsi_endpoint *beiscsi_ep, int flag)
        return ret;
 }
 
-/**
- * beiscsi_ep_disconnect - Tears down the TCP connection
- * @ep:        endpoint to be used
- *
- * Tears down the TCP connection
- */
-void beiscsi_ep_disconnect(struct iscsi_endpoint *ep)
-{
-       struct beiscsi_conn *beiscsi_conn;
-       struct beiscsi_endpoint *beiscsi_ep;
-       struct beiscsi_hba *phba;
-
-       beiscsi_ep = ep->dd_data;
-       phba = beiscsi_ep->phba;
-       SE_DEBUG(DBG_LVL_8, "In beiscsi_ep_disconnect for ep_cid = %d\n",
-                            beiscsi_ep->ep_cid);
-
-       if (beiscsi_ep->conn) {
-               beiscsi_conn = beiscsi_ep->conn;
-               iscsi_suspend_queue(beiscsi_conn->conn);
-       }
-
-}
-
 /**
  * beiscsi_unbind_conn_to_cid - Unbind the beiscsi_conn from phba conn table
  * @phba: The phba instance
@@ -673,28 +653,35 @@ static int beiscsi_unbind_conn_to_cid(struct beiscsi_hba *phba,
 }
 
 /**
- * beiscsi_conn_stop - Invalidate and stop the connection
- * @cls_conn: pointer to get iscsi_conn
- * @flag: The type of connection closure
+ * beiscsi_ep_disconnect - Tears down the TCP connection
+ * @ep:        endpoint to be used
+ *
+ * Tears down the TCP connection
  */
-void beiscsi_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)
+void beiscsi_ep_disconnect(struct iscsi_endpoint *ep)
 {
-       struct iscsi_conn *conn = cls_conn->dd_data;
-       struct beiscsi_conn *beiscsi_conn = conn->dd_data;
+       struct beiscsi_conn *beiscsi_conn;
        struct beiscsi_endpoint *beiscsi_ep;
-       struct iscsi_session *session = conn->session;
-       struct Scsi_Host *shost = iscsi_session_to_shost(session->cls_session);
-       struct beiscsi_hba *phba = iscsi_host_priv(shost);
+       struct beiscsi_hba *phba;
        unsigned int tag;
        unsigned short savecfg_flag = CMD_ISCSI_SESSION_SAVE_CFG_ON_FLASH;
 
-       beiscsi_ep = beiscsi_conn->ep;
-       if (!beiscsi_ep) {
-               SE_DEBUG(DBG_LVL_8, "In beiscsi_conn_stop , no beiscsi_ep\n");
+       beiscsi_ep = ep->dd_data;
+       phba = beiscsi_ep->phba;
+       SE_DEBUG(DBG_LVL_8, "In beiscsi_ep_disconnect for ep_cid = %d\n",
+                            beiscsi_ep->ep_cid);
+
+       if (!beiscsi_ep->conn) {
+               SE_DEBUG(DBG_LVL_8, "In beiscsi_ep_disconnect, no "
+                        "beiscsi_ep\n");
                return;
        }
-       SE_DEBUG(DBG_LVL_8, "In beiscsi_conn_stop  ep_cid = %d\n",
-                            beiscsi_ep->ep_cid);
+       beiscsi_conn = beiscsi_ep->conn;
+       iscsi_suspend_queue(beiscsi_conn->conn);
+
+       SE_DEBUG(DBG_LVL_8, "In beiscsi_ep_disconnect ep_cid = %d\n",
+                beiscsi_ep->ep_cid);
+
        tag = mgmt_invalidate_connection(phba, beiscsi_ep,
                                            beiscsi_ep->ep_cid, 1,
                                            savecfg_flag);
@@ -707,9 +694,9 @@ void beiscsi_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)
                                         phba->ctrl.mcc_numtag[tag]);
                free_mcc_tag(&phba->ctrl, tag);
        }
+
        beiscsi_close_conn(beiscsi_ep, CONNECTION_UPLOAD_GRACEFUL);
        beiscsi_free_ep(beiscsi_ep);
-       iscsi_destroy_endpoint(beiscsi_ep->openiscsi_ep);
        beiscsi_unbind_conn_to_cid(phba, beiscsi_ep->ep_cid);
-       iscsi_conn_stop(cls_conn, flag);
+       iscsi_destroy_endpoint(beiscsi_ep->openiscsi_ep);
 }
index 1f512c28cbf942da401d9cec1df3e7b6b74abcc8..870cdb2a73e45070caa50e679c89b2ff7c9c9ed2 100644 (file)
@@ -59,8 +59,6 @@ int beiscsi_set_param(struct iscsi_cls_conn *cls_conn,
 
 int beiscsi_conn_start(struct iscsi_cls_conn *cls_conn);
 
-void beiscsi_conn_stop(struct iscsi_cls_conn *cls_conn, int flag);
-
 struct iscsi_endpoint *beiscsi_ep_connect(struct Scsi_Host *shost,
                                          struct sockaddr *dst_addr,
                                          int non_blocking);
index dd5b105f8f47a73ab45e3c73e43a2f9f9353b52a..854414551bb07efa1b5279a818ee153f1438fd92 100644 (file)
@@ -3955,7 +3955,7 @@ struct iscsi_transport beiscsi_iscsi_transport = {
        .get_session_param = iscsi_session_get_param,
        .get_host_param = beiscsi_get_host_param,
        .start_conn = beiscsi_conn_start,
-       .stop_conn = beiscsi_conn_stop,
+       .stop_conn = iscsi_conn_stop,
        .send_pdu = iscsi_conn_send_pdu,
        .xmit_task = beiscsi_task_xmit,
        .cleanup_task = beiscsi_cleanup_task,