iscsi: data digest page cache usage fix

PATCH Superseded Mike Christie 2 participants 3 messages Jan 13, 2006

Subsystems (2)

SCSI iSCSI

lore

Share & tools

lore

Mike Christie

author

2006-01-13 05:08 UTC #0 lore raw

PATCH 4/10 iscsi: data digest page cache usage fix

Quoted 1 line

From FUJITA Tomonori <[email protected]>:

Users can write to a page while we are sending it and making digest calculations. This ends up causing us to retry the command when a digest error is later reported. By using sock_no_sendpage when data digests are calculated we can avoid a lot of (not all but it helps) the retries becuase sock_no_sendpage is not zero copy.

Signed-off-by Alex Aizman <[email protected]>

Signed-off-by Dmitry Yusupov <[email protected]>

Signed-off-by Mike Christie <[email protected]>

drivers/scsi/iscsi_tcp.c +9 -7

index cd1491e..5e8b313 100644
@@ -1318,15 +1318,15 @@ iscsi_conn_restore_callbacks(struct iscs
  *	to use tcp_sendmsg().
  */
 static inline int
-iscsi_send(struct socket *sk, struct iscsi_buf *buf, int size, int flags)
+iscsi_send(struct iscsi_conn *conn, struct iscsi_buf *buf, int size, int flags)
 {
+	struct socket *sk = conn->sock;
 	int res;
 
 	if ((int)buf->sg.offset >= 0) {
 		int offset = buf->sg.offset + buf->sent;
 
-		/* tcp_sendpage */
-		res = sk->ops->sendpage(sk, buf->sg.page, offset, size, flags);
+		res = conn->sendpage(sk, buf->sg.page, offset, size, flags);
 	} else {
 		struct msghdr msg;

@@ -1354,7 +1354,6 @@ iscsi_send(struct socket *sk, struct isc
 static inline int
 iscsi_sendhdr(struct iscsi_conn *conn, struct iscsi_buf *buf, int datalen)
 {
-	struct socket *sk = conn->sock;
 	int flags = 0; /* MSG_DONTWAIT; */
 	int res, size;

@@ -1363,7 +1362,7 @@ iscsi_sendhdr(struct iscsi_conn *conn, s
 	if (buf->sent + size != buf->sg.length || datalen)
 		flags |= MSG_MORE;
 
-	res = iscsi_send(sk, buf, size, flags);
+	res = iscsi_send(conn, buf, size, flags);
 	debug_tcp("sendhdr %d bytes, sent %d res %d\n", size, buf->sent, res);
 	if (res >= 0) {
 		conn->txdata_octets += res;

@@ -1394,7 +1393,6 @@ static inline int
 iscsi_sendpage(struct iscsi_conn *conn, struct iscsi_buf *buf,
 	       int *count, int *sent)
 {
-	struct socket *sk = conn->sock;
 	int flags = 0; /* MSG_DONTWAIT; */
 	int res, size;

@@ -1405,7 +1403,7 @@ iscsi_sendpage(struct iscsi_conn *conn, 
 	if (buf->sent + size != buf->sg.length || *count != size)
 		flags |= MSG_MORE;
 
-	res = iscsi_send(sk, buf, size, flags);
+	res = iscsi_send(conn, buf, size, flags);
 	debug_tcp("sendpage: %d bytes, sent %d left %d sent %d res %d\n",
 		  size, buf->sent, *count, *sent, res);
 	if (res >= 0) {

@@ -2713,6 +2711,8 @@ iscsi_conn_bind(iscsi_sessionh_t session
 		 */
 		iscsi_conn_set_callbacks(conn);
 
+		conn->sendpage = conn->sock->ops->sendpage;
+
 		/*
 		 * set receive state machine into initial state
 		 */

@@ -3467,6 +3467,8 @@ iscsi_conn_set_param(iscsi_connh_t connh
 			if (conn->data_rx_tfm)
 				crypto_free_tfm(conn->data_rx_tfm);
 		}
+		conn->sendpage = conn->datadgst_en ?
+			sock_no_sendpage : conn->sock->ops->sendpage;
 		break;
 	case ISCSI_PARAM_INITIAL_R2T_EN:
 		session->initial_r2t_en = value;

drivers/scsi/iscsi_tcp.h +2 -0

index 9badafe..c8bb5b0 100644
@@ -191,6 +191,8 @@ struct iscsi_conn {
 	uint32_t		sendpage_failures_cnt;
 	uint32_t		discontiguous_hdr_cnt;
 	uint32_t		eh_abort_cnt;
+
+	ssize_t (*sendpage)(struct socket *, struct page *, int, size_t, int);
 };
 
 struct iscsi_session {

Christoph Hellwig

2006-01-13 18:14 UTC #1 lore raw

Quoted 7 lines · Mike Christie

Quoted 1 line
From FUJITA Tomonori <[email protected]>:
Users can write to a page while we are sending it and making digest calculations. This ends up causing us to retry the command when a digest error is later reported. By using sock_no_sendpage when data digests are calculated we can avoid a lot of (not all but it helps) the retries becuase sock_no_sendpage is not zero copy.

This sounds like it'd kill performance badly when using data digests. But using lock_page from an LLDD sounds pretty bad aswell, so I don't know anything better.

Mike Christie

author

2006-01-13 18:19 UTC #2 lore raw

Quoted 11 lines · Christoph Hellwig

Quoted 7 lines · Mike Christie
Quoted 1 line
From FUJITA Tomonori <[email protected]>:
Users can write to a page while we are sending it and making digest calculations. This ends up causing us to retry the command when a digest error is later reported. By using sock_no_sendpage when data digests are calculated we can avoid a lot of (not all but it helps) the retries becuase sock_no_sendpage is not zero copy.

Quoted 4 lines

This sounds like it'd kill performance badly when using data digests. But using lock_page from an LLDD sounds pretty bad aswell, so I don't know anything better.

Yeah, we kinda accpeted that if you use data digests in linux performance will drop. It will either drop due to the copy we added here or due to the mutiple digest errors/retry path we will hit. Either way, the user should know that safety they get from data digests comes at a price.