On 03/11/2013 12:16 PM, Alex Elder wrote:
On 03/11/2013 01:57 PM, Josh Durgin wrote:
On 03/09/2013 07:14 AM, Alex Elder wrote:
Define a new function in_msg_pos_next() to match out_msg_pos_next(),
and use it in place of code at the end of read_partial_message_pages()
and read_partial_message_bio().
Signed-off-by: Alex Elder <elder@xxxxxxxxxxx>
---
net/ceph/messenger.c | 57
++++++++++++++++++++++++++++++++------------------
1 file changed, 37 insertions(+), 20 deletions(-)
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 2017b88..fb5f6e7 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -1052,6 +1052,28 @@ static void out_msg_pos_next(struct
ceph_connection *con, struct page *page,
#endif
}
+static void in_msg_pos_next(struct ceph_connection *con, size_t len,
+ size_t received)
+{
+ struct ceph_msg *msg = con->in_msg;
+
+ BUG_ON(!msg);
+ BUG_ON(!received);
+
+ con->in_msg_pos.data_pos += received;
+ con->in_msg_pos.page_pos += received;
+ if (received < len)
+ return;
This is different from the condition in read_partial_message_pages()
case, which only increments in_msg_pos.page and resets page_pos when
page_pos == PAGE_SIZE. Maybe this is equivalent, but if so it's not
obvious.
You are correct. And, despite my usual thoroughness and
verbosity, I didn't explain this one.
This message position structure (in_msg_pos) is tracking
how much of the message has been consumed. Each time a
an incoming message is going to arrive, we find out how
much room is left--not surpassing the current page--and
provide that as the "len" (number of bytes) to receive:
left = min((int)(data_len - con->in_msg_pos.data_pos),
(int)(bv->bv_len - con->in_msg_pos.page_pos));
This is saying the amount we'll use is the lesser of:
- all that's left of the entire request
- all that's left in the current page
The number of bytes received is then used, along with
the "len" value, to determine how to advance the position.
If we received exactly how many were requested, we either
reached the end of the request or the end of the page.
In the first case, we're done, in the second, we move
onto the next page in the array.
In all cases but (possibly) on the last page, after adding
the number of bytes received, page_pos == PAGE_SIZE. On the
last page, it doesn't really matter whether we increment
the page number and reset the page position, because we're
done and we won't come back here again. The code previously
skipped over that last case, basically. The new code
handles that case the same as the others, incrementing
and resetting.
So the short answer is, they are still equivalent. And
I agree, it is not obvious.
If you are satisfied, I can add the above (or maybe an
abbreviated form of it) to the commit message.
Thanks for the explanation, that makes sense now.
It'd be nice to have some form of it in the commit message.
Reviewed-by: Josh Durgin <josh.durgin@xxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
