libceph: more miscellaneous cleanups

PATCH In review Alex Elder 2 participants 5 messages Feb 29, 2012 – Mar 2, 2012 Ceph common code (libceph)

Share & tools

Alex Elder

author

2012-02-29 04:52 UTC #0 lore raw

cover libceph: more miscellaneous cleanups

Messaging code again. The biggest improvement here is encapsulating the code that puts data into the write vectors. The second patch just adds more simple cleanups.

-Alex

Alex Elder

author

2012-02-29 04:55 UTC #1 lore raw

Quoted 3 lines · Alex Elder

Messaging code again. The biggest improvement here is encapsulating the code that puts data into the write vectors. The second patch just adds more simple cleanups.

What I meant to say was...

These are two simple patches, the first is just a simple logic change whose result I thought was slightly more readable.

-Alex

Alex Elder

author

2012-02-29 04:55 UTC #2 lore raw

PATCH 1/2 libceph: small refactor in write_partial_kvec()

In reply to Alex Elder

Make a small change in the code that counts down kvecs consumed by a ceph_tcp_sendmsg() call. Same functionality, just blocked out a little differently.

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

---
 net/ceph/messenger.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

net/ceph/messenger.c +12 ~~-11~~

index 693be64..63f281f 100644
@@ -744,17 +744,18 @@ static int write_partial_kvec(struct ceph_connection *con)
 		con->out_kvec_bytes -= ret;
 		if (con->out_kvec_bytes == 0)
 			break;            /* done */
-		while (ret > 0) {
-			if (ret >= con->out_kvec_cur->iov_len) {
-				ret -= con->out_kvec_cur->iov_len;
-				con->out_kvec_cur++;
-				con->out_kvec_left--;
-			} else {
-				con->out_kvec_cur->iov_len -= ret;
-				con->out_kvec_cur->iov_base += ret;
-				ret = 0;
-				break;
-			}
+
+		/* account for full iov entries consumed */
+		while (ret >= con->out_kvec_cur->iov_len) {
+			BUG_ON(!con->out_kvec_left);
+			ret -= con->out_kvec_cur->iov_len;
+			con->out_kvec_cur++;
+			con->out_kvec_left--;
+		}
+		/* and for a partially-consumed entry */
+		if (ret) {
+			con->out_kvec_cur->iov_len -= ret;
+			con->out_kvec_cur->iov_base += ret;
 		}
 	}
 	con->out_kvec_left = 0;

Signature · 1 line

1.7.5.4

Alex Elder

author

2012-02-29 04:55 UTC #3 lore raw

PATCH 2/2 libceph: some simple changes

In reply to Alex Elder

Nothing too big here.
- define the size of the buffer used for consuming ignored incoming data using a symbolic constant
- simplify the condition determining whether to unmap the page in write_partial_msg_pages(): do it for crc but not if the page is the zero page

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

---
 net/ceph/messenger.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

net/ceph/messenger.c +9 -4

index 63f281f..27a1364 100644
@@ -38,6 +38,11 @@ static char tag_keepalive = CEPH_MSGR_TAG_KEEPALIVE;
 static struct lock_class_key socket_class;
 #endif
 
+/*
+ * When skipping (ignoring) a block of input we read it into a "skip
+ * buffer," which is this many bytes in size.
+ */
+#define SKIP_BUF_SIZE	1024
 
 static void queue_con(struct ceph_connection *con);
 static void con_work(struct work_struct *);

@@ -889,8 +894,7 @@ static int write_partial_msg_pages(struct ceph_connection *con)
 				      MSG_DONTWAIT | MSG_NOSIGNAL |
 				      MSG_MORE);
 
-		if (do_crc &&
-		    (msg->pages || msg->pagelist || msg->bio || in_trail))
+		if (do_crc && kaddr != zero_page_address)
 			kunmap(page);
 
 		if (ret == -EAGAIN)

@@ -1979,8 +1983,9 @@ more:
 		 *
 		 * FIXME: there must be a better way to do this!
 		 */
-		static char buf[1024];
-		int skip = min(1024, -con->in_base_pos);
+		static char buf[SKIP_BUF_SIZE];
+		int skip = min((int) sizeof buf, -con->in_base_pos);
+
 		dout("skipping %d / %d bytes\n", skip, -con->in_base_pos);
 		ret = ceph_tcp_recvmsg(con->sock, buf, skip);
 		if (ret <= 0)

Signature · 1 line

1.7.5.4

Sage Weil

Reviewed-by on v1 2/2

2012-03-02 21:49 UTC #4 lore raw

Re: [PATCH 2/2] libceph: some simple changes

Looks ok, as long as this isn't broken if the moving zero_page_address inside write_partial_msg_pages().

Reviewed-by Sage Weil <[email protected]>

Quoted 60 lines · Alex Elder

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

---
 net/ceph/messenger.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

net/ceph/messenger.c +6 -2

index 63f281f..27a1364 100644
@@ -38,6 +38,11 @@ static char tag_keepalive = CEPH_MSGR_TAG_KEEPALIVE;
 static struct lock_class_key socket_class;
 #endif
 
+/*
+ * When skipping (ignoring) a block of input we read it into a "skip
+ * buffer," which is this many bytes in size.
+ */
+#define SKIP_BUF_SIZE	1024
 
 static void queue_con(struct ceph_connection *con);
 static void con_work(struct work_struct *);

@@ -889,8 +894,7 @@ static int write_partial_msg_pages(struct ceph_connection
 *con)
 				      MSG_DONTWAIT | MSG_NOSIGNAL |
 				      MSG_MORE);
 
-		if (do_crc &&
-		    (msg->pages || msg->pagelist || msg->bio || in_trail))
+		if (do_crc && kaddr != zero_page_address)
 			kunmap(page);

net/ceph/messenger.c +3 -2

 		if (ret == -EAGAIN)

@@ -1979,8 +1983,9 @@ more:
 		 *
 		 * FIXME: there must be a better way to do this!
 		 */
-		static char buf[1024];
-		int skip = min(1024, -con->in_base_pos);
+		static char buf[SKIP_BUF_SIZE];
+		int skip = min((int) sizeof buf, -con->in_base_pos);
+
 		dout("skipping %d / %d bytes\n", skip, -con->in_base_pos);
 		ret = ceph_tcp_recvmsg(con->sock, buf, skip);
 		if (ret <= 0)

Signature · 6 lines

1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html