[SCSI] libcxgbi: pdu read fixes

Fixed the locking and releasing skb in the case of error in the pdu read path, and added define iscsi_task_cxgbi_data to access the private data inside the iscsi_task. Signed-off-by: Karen Xie <kxie@chelsio.com> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu> Signed-off-by: James Bottomley <James.Bottomley@suse.de>

[SCSI] libcxgbi: pdu read fixes
Fixed the locking and releasing skb in the case of error in the pdu read path, and added define iscsi_task_cxgbi_data to access the private data inside the iscsi_task. Signed-off-by: Karen Xie <kxie@chelsio.com> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu> Signed-off-by: James Bottomley <James.Bottomley@suse.de>
kxie@chelsio.com · James Bottomley
1 parent 24d3f95a5b
Showing 2 changed files with 53 additions and 28 deletions Side-by-side Diff
drivers/scsi/cxgbi/libcxgbi.c
drivers/scsi/cxgbi/libcxgbi.h
@@ -593,11 +593,11 @@
 		csk, csk->state, csk->flags, csk->user_data);
  
 	if (csk->state != CTP_ESTABLISHED) {
-		read_lock(&csk->callback_lock);
+		read_lock_bh(&csk->callback_lock);
 		if (csk->user_data)
 			iscsi_conn_failure(csk->user_data,
 					ISCSI_ERR_CONN_FAILED);
-		read_unlock(&csk->callback_lock);
+		read_unlock_bh(&csk->callback_lock);
 	}
 }
  
  
@@ -1712,12 +1712,10 @@
 			"csk 0x%p, conn 0x%p, id %d, suspend_rx %lu!\n",
 			csk, conn, conn ? conn->id : 0xFF,
 			conn ? conn->suspend_rx : 0xFF);
-		read_unlock(&csk->callback_lock);
 		return;
 	}
  
 	while (!err) {
-		read_lock(&csk->callback_lock);
 		skb = skb_peek(&csk->receive_queue);
 		if (!skb ||
 		    !(cxgbi_skcb_test_flag(skb, SKCBF_RX_STATUS))) {
  
@@ -1725,11 +1723,9 @@
 				log_debug(1 << CXGBI_DBG_PDU_RX,
 					"skb 0x%p, NOT ready 0x%lx.\n",
 					skb, cxgbi_skcb_flags(skb));
-			read_unlock(&csk->callback_lock);
 			break;
 		}
 		__skb_unlink(skb, &csk->receive_queue);
-		read_unlock(&csk->callback_lock);
  
 		read += cxgbi_skcb_rx_pdulen(skb);
 		log_debug(1 << CXGBI_DBG_PDU_RX,
  
  
  
  
  
  
  
  
@@ -1739,37 +1735,66 @@
  
 		if (cxgbi_skcb_test_flag(skb, SKCBF_RX_COALESCED)) {
 			err = skb_read_pdu_bhs(conn, skb);
-			if (err < 0)
-				break;
+			if (err < 0) {
+				pr_err("coalesced bhs, csk 0x%p, skb 0x%p,%u, "
+					"f 0x%lx, plen %u.\n",
+					csk, skb, skb->len,
+					cxgbi_skcb_flags(skb),
+					cxgbi_skcb_rx_pdulen(skb));
+				goto skb_done;
+			}
 			err = skb_read_pdu_data(conn, skb, skb,
 						err + cdev->skb_rx_extra);
+			if (err < 0)
+				pr_err("coalesced data, csk 0x%p, skb 0x%p,%u, "
+					"f 0x%lx, plen %u.\n",
+					csk, skb, skb->len,
+					cxgbi_skcb_flags(skb),
+					cxgbi_skcb_rx_pdulen(skb));
 		} else {
 			err = skb_read_pdu_bhs(conn, skb);
-			if (err < 0)
-				break;
+			if (err < 0) {
+				pr_err("bhs, csk 0x%p, skb 0x%p,%u, "
+					"f 0x%lx, plen %u.\n",
+					csk, skb, skb->len,
+					cxgbi_skcb_flags(skb),
+					cxgbi_skcb_rx_pdulen(skb));
+				goto skb_done;
+			}
+
 			if (cxgbi_skcb_test_flag(skb, SKCBF_RX_DATA)) {
 				struct sk_buff *dskb;
  
-				read_lock(&csk->callback_lock);
 				dskb = skb_peek(&csk->receive_queue);
 				if (!dskb) {
-					read_unlock(&csk->callback_lock);
-					pr_err("csk 0x%p, NO data.\n", csk);
-					err = -EAGAIN;
-					break;
+					pr_err("csk 0x%p, skb 0x%p,%u, f 0x%lx,"
+						" plen %u, NO data.\n",
+						csk, skb, skb->len,
+						cxgbi_skcb_flags(skb),
+						cxgbi_skcb_rx_pdulen(skb));
+					err = -EIO;
+					goto skb_done;
 				}
 				__skb_unlink(dskb, &csk->receive_queue);
-				read_unlock(&csk->callback_lock);
  
 				err = skb_read_pdu_data(conn, skb, dskb, 0);
+				if (err < 0)
+					pr_err("data, csk 0x%p, skb 0x%p,%u, "
+						"f 0x%lx, plen %u, dskb 0x%p,"
+						"%u.\n",
+						csk, skb, skb->len,
+						cxgbi_skcb_flags(skb),
+						cxgbi_skcb_rx_pdulen(skb),
+						dskb, dskb->len);
 				__kfree_skb(dskb);
 			} else
 				err = skb_read_pdu_data(conn, skb, skb, 0);
 		}
+skb_done:
+		__kfree_skb(skb);
+
 		if (err < 0)
 			break;
-
-		__kfree_skb(skb);
 	}
  
 	log_debug(1 << CXGBI_DBG_PDU_RX, "csk 0x%p, read %u.\n", csk, read);
@@ -1780,7 +1805,8 @@
 	}
  
 	if (err < 0) {
-		pr_info("csk 0x%p, 0x%p, rx failed %d.\n", csk, conn, err);
+		pr_info("csk 0x%p, 0x%p, rx failed %d, read %u.\n",
+			csk, conn, err, read);
 		iscsi_conn_failure(conn, ISCSI_ERR_CONN_FAILED);
 	}
 }
@@ -1861,7 +1887,7 @@
 	struct cxgbi_device *cdev = cconn->chba->cdev;
 	struct iscsi_conn *conn = task->conn;
 	struct iscsi_tcp_task *tcp_task = task->dd_data;
-	struct cxgbi_task_data *tdata = task->dd_data + sizeof(*tcp_task);
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 	struct scsi_cmnd *sc = task->sc;
 	int headroom = SKB_TX_ISCSI_PDU_HEADER_MAX;
  
@@ -1916,8 +1942,7 @@
 			      unsigned int count)
 {
 	struct iscsi_conn *conn = task->conn;
-	struct iscsi_tcp_task *tcp_task = task->dd_data;
-	struct cxgbi_task_data *tdata = tcp_task->dd_data;
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 	struct sk_buff *skb = tdata->skb;
 	unsigned int datalen = count;
 	int i, padlen = iscsi_padding(count);
@@ -2019,8 +2044,7 @@
 {
 	struct iscsi_tcp_conn *tcp_conn = task->conn->dd_data;
 	struct cxgbi_conn *cconn = tcp_conn->dd_data;
-	struct iscsi_tcp_task *tcp_task = task->dd_data;
-	struct cxgbi_task_data *tdata = tcp_task->dd_data;
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 	struct sk_buff *skb = tdata->skb;
 	unsigned int datalen;
 	int err;
@@ -2072,8 +2096,7 @@
  
 void cxgbi_cleanup_task(struct iscsi_task *task)
 {
-	struct cxgbi_task_data *tdata = task->dd_data +
-				sizeof(struct iscsi_tcp_task);
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
  
 	log_debug(1 << CXGBI_DBG_ISCSI,
 		"task 0x%p, skb 0x%p, itt 0x%x.\n",
  
@@ -2290,12 +2313,12 @@
 	/*  calculate the tag idx bits needed for this conn based on cmds_max */
 	cconn->task_idx_bits = (__ilog2_u32(conn->session->cmds_max - 1)) + 1;
  
-	write_lock(&csk->callback_lock);
+	write_lock_bh(&csk->callback_lock);
 	csk->user_data = conn;
 	cconn->chba = cep->chba;
 	cconn->cep = cep;
 	cep->cconn = cconn;
-	write_unlock(&csk->callback_lock);
+	write_unlock_bh(&csk->callback_lock);
  
 	cxgbi_conn_max_xmit_dlength(conn);
 	cxgbi_conn_max_recv_dlength(conn);
@@ -592,6 +592,8 @@
 	unsigned int count;
 	unsigned int sgoffset;
 };
+#define iscsi_task_cxgbi_data(task) \
+	((task)->dd_data + sizeof(struct iscsi_tcp_task))
  
 static inline int cxgbi_is_ddp_tag(struct cxgbi_tag_format *tformat, u32 tag)
 {
...	...	@@ -593,11 +593,11 @@
593	593	csk, csk->state, csk->flags, csk->user_data);
594	594
595	595	if (csk->state != CTP_ESTABLISHED) {
596		- read_lock(&csk->callback_lock);
	596	+ read_lock_bh(&csk->callback_lock);
597	597	if (csk->user_data)
598	598	iscsi_conn_failure(csk->user_data,
599	599	ISCSI_ERR_CONN_FAILED);
600		- read_unlock(&csk->callback_lock);
	600	+ read_unlock_bh(&csk->callback_lock);
601	601	}
602	602	}
603	603
604	604
...	...	@@ -1712,12 +1712,10 @@
1712	1712	"csk 0x%p, conn 0x%p, id %d, suspend_rx %lu!\n",
1713	1713	csk, conn, conn ? conn->id : 0xFF,
1714	1714	conn ? conn->suspend_rx : 0xFF);
1715		- read_unlock(&csk->callback_lock);
1716	1715	return;
1717	1716	}
1718	1717
1719	1718	while (!err) {
1720		- read_lock(&csk->callback_lock);
1721	1719	skb = skb_peek(&csk->receive_queue);
1722	1720	if (!skb \|\|
1723	1721	!(cxgbi_skcb_test_flag(skb, SKCBF_RX_STATUS))) {
1724	1722
...	...	@@ -1725,11 +1723,9 @@
1725	1723	log_debug(1 << CXGBI_DBG_PDU_RX,
1726	1724	"skb 0x%p, NOT ready 0x%lx.\n",
1727	1725	skb, cxgbi_skcb_flags(skb));
1728		- read_unlock(&csk->callback_lock);
1729	1726	break;
1730	1727	}
1731	1728	__skb_unlink(skb, &csk->receive_queue);
1732		- read_unlock(&csk->callback_lock);
1733	1729
1734	1730	read += cxgbi_skcb_rx_pdulen(skb);
1735	1731	log_debug(1 << CXGBI_DBG_PDU_RX,
1736	1732
1737	1733
1738	1734
1739	1735
1740	1736
1741	1737
1742	1738
1743	1739
...	...	@@ -1739,37 +1735,66 @@
1739	1735
1740	1736	if (cxgbi_skcb_test_flag(skb, SKCBF_RX_COALESCED)) {
1741	1737	err = skb_read_pdu_bhs(conn, skb);
1742		- if (err < 0)
1743		- break;
	1738	+ if (err < 0) {
	1739	+ pr_err("coalesced bhs, csk 0x%p, skb 0x%p,%u, "
	1740	+ "f 0x%lx, plen %u.\n",
	1741	+ csk, skb, skb->len,
	1742	+ cxgbi_skcb_flags(skb),
	1743	+ cxgbi_skcb_rx_pdulen(skb));
	1744	+ goto skb_done;
	1745	+ }
1744	1746	err = skb_read_pdu_data(conn, skb, skb,
1745	1747	err + cdev->skb_rx_extra);
	1748	+ if (err < 0)
	1749	+ pr_err("coalesced data, csk 0x%p, skb 0x%p,%u, "
	1750	+ "f 0x%lx, plen %u.\n",
	1751	+ csk, skb, skb->len,
	1752	+ cxgbi_skcb_flags(skb),
	1753	+ cxgbi_skcb_rx_pdulen(skb));
1746	1754	} else {
1747	1755	err = skb_read_pdu_bhs(conn, skb);
1748		- if (err < 0)
1749		- break;
	1756	+ if (err < 0) {
	1757	+ pr_err("bhs, csk 0x%p, skb 0x%p,%u, "
	1758	+ "f 0x%lx, plen %u.\n",
	1759	+ csk, skb, skb->len,
	1760	+ cxgbi_skcb_flags(skb),
	1761	+ cxgbi_skcb_rx_pdulen(skb));
	1762	+ goto skb_done;
	1763	+ }
	1764	+
1750	1765	if (cxgbi_skcb_test_flag(skb, SKCBF_RX_DATA)) {
1751	1766	struct sk_buff *dskb;
1752	1767
1753		- read_lock(&csk->callback_lock);
1754	1768	dskb = skb_peek(&csk->receive_queue);
1755	1769	if (!dskb) {
1756		- read_unlock(&csk->callback_lock);
1757		- pr_err("csk 0x%p, NO data.\n", csk);
1758		- err = -EAGAIN;
1759		- break;
	1770	+ pr_err("csk 0x%p, skb 0x%p,%u, f 0x%lx,"
	1771	+ " plen %u, NO data.\n",
	1772	+ csk, skb, skb->len,
	1773	+ cxgbi_skcb_flags(skb),
	1774	+ cxgbi_skcb_rx_pdulen(skb));
	1775	+ err = -EIO;
	1776	+ goto skb_done;
1760	1777	}
1761	1778	__skb_unlink(dskb, &csk->receive_queue);
1762		- read_unlock(&csk->callback_lock);
1763	1779
1764	1780	err = skb_read_pdu_data(conn, skb, dskb, 0);
	1781	+ if (err < 0)
	1782	+ pr_err("data, csk 0x%p, skb 0x%p,%u, "
	1783	+ "f 0x%lx, plen %u, dskb 0x%p,"
	1784	+ "%u.\n",
	1785	+ csk, skb, skb->len,
	1786	+ cxgbi_skcb_flags(skb),
	1787	+ cxgbi_skcb_rx_pdulen(skb),
	1788	+ dskb, dskb->len);
1765	1789	__kfree_skb(dskb);
1766	1790	} else
1767	1791	err = skb_read_pdu_data(conn, skb, skb, 0);
1768	1792	}
	1793	+skb_done:
	1794	+ __kfree_skb(skb);
	1795	+
1769	1796	if (err < 0)
1770	1797	break;
1771		-
1772		- __kfree_skb(skb);
1773	1798	}
1774	1799
1775	1800	log_debug(1 << CXGBI_DBG_PDU_RX, "csk 0x%p, read %u.\n", csk, read);
...	...	@@ -1780,7 +1805,8 @@
1780	1805	}
1781	1806
1782	1807	if (err < 0) {
1783		- pr_info("csk 0x%p, 0x%p, rx failed %d.\n", csk, conn, err);
	1808	+ pr_info("csk 0x%p, 0x%p, rx failed %d, read %u.\n",
	1809	+ csk, conn, err, read);
1784	1810	iscsi_conn_failure(conn, ISCSI_ERR_CONN_FAILED);
1785	1811	}
1786	1812	}
...	...	@@ -1861,7 +1887,7 @@
1861	1887	struct cxgbi_device *cdev = cconn->chba->cdev;
1862	1888	struct iscsi_conn *conn = task->conn;
1863	1889	struct iscsi_tcp_task *tcp_task = task->dd_data;
1864		- struct cxgbi_task_data tdata = task->dd_data + sizeof(tcp_task);
	1890	+ struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
1865	1891	struct scsi_cmnd *sc = task->sc;
1866	1892	int headroom = SKB_TX_ISCSI_PDU_HEADER_MAX;
1867	1893
...	...	@@ -1916,8 +1942,7 @@
1916	1942	unsigned int count)
1917	1943	{
1918	1944	struct iscsi_conn *conn = task->conn;
1919		- struct iscsi_tcp_task *tcp_task = task->dd_data;
1920		- struct cxgbi_task_data *tdata = tcp_task->dd_data;
	1945	+ struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
1921	1946	struct sk_buff *skb = tdata->skb;
1922	1947	unsigned int datalen = count;
1923	1948	int i, padlen = iscsi_padding(count);
...	...	@@ -2019,8 +2044,7 @@
2019	2044	{
2020	2045	struct iscsi_tcp_conn *tcp_conn = task->conn->dd_data;
2021	2046	struct cxgbi_conn *cconn = tcp_conn->dd_data;
2022		- struct iscsi_tcp_task *tcp_task = task->dd_data;
2023		- struct cxgbi_task_data *tdata = tcp_task->dd_data;
	2047	+ struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
2024	2048	struct sk_buff *skb = tdata->skb;
2025	2049	unsigned int datalen;
2026	2050	int err;
...	...	@@ -2072,8 +2096,7 @@
2072	2096
2073	2097	void cxgbi_cleanup_task(struct iscsi_task *task)
2074	2098	{
2075		- struct cxgbi_task_data *tdata = task->dd_data +
2076		- sizeof(struct iscsi_tcp_task);
	2099	+ struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
2077	2100
2078	2101	log_debug(1 << CXGBI_DBG_ISCSI,
2079	2102	"task 0x%p, skb 0x%p, itt 0x%x.\n",
2080	2103
...	...	@@ -2290,12 +2313,12 @@
2290	2313	/* calculate the tag idx bits needed for this conn based on cmds_max */
2291	2314	cconn->task_idx_bits = (__ilog2_u32(conn->session->cmds_max - 1)) + 1;
2292	2315
2293		- write_lock(&csk->callback_lock);
	2316	+ write_lock_bh(&csk->callback_lock);
2294	2317	csk->user_data = conn;
2295	2318	cconn->chba = cep->chba;
2296	2319	cconn->cep = cep;
2297	2320	cep->cconn = cconn;
2298		- write_unlock(&csk->callback_lock);
	2321	+ write_unlock_bh(&csk->callback_lock);
2299	2322
2300	2323	cxgbi_conn_max_xmit_dlength(conn);
2301	2324	cxgbi_conn_max_recv_dlength(conn);
...	...	@@ -592,6 +592,8 @@
592	592	unsigned int count;
593	593	unsigned int sgoffset;
594	594	};
	595	+#define iscsi_task_cxgbi_data(task) \
	596	+ ((task)->dd_data + sizeof(struct iscsi_tcp_task))
595	597
596	598	static inline int cxgbi_is_ddp_tag(struct cxgbi_tag_format *tformat, u32 tag)
597	599	{