aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlex Elder <elder@inktank.com>2013-04-19 16:34:49 -0400
committerSage Weil <sage@inktank.com>2013-05-02 00:19:05 -0400
commita51b272e9e99f912e8e07d4c9f58c1d433afea7c (patch)
tree2ddc1a7fcae1585618d192e5ab8ddfa4b41200bf
parentc5b5ef6c51124e61829632251098f8b5efecae8a (diff)
libceph: fix two messenger bugs
This patch makes four small changes in the ceph messenger. While getting copyup functionality working I found two bugs in the messenger. Existing paths through the code did not trigger these problems, but they're fixed here: - In ceph_msg_data_pagelist_cursor_init(), the cursor's last_piece field was being checked against the length supplied. This was OK until this commit: ccba6d98 libceph: implement multiple data items in a message That commit changed the cursor init routines to allow lengths to be supplied that exceeded the size of the current data item. Because of this, we have to use the assigned cursor resid field rather than the provided length in determining whether the cursor points to the last piece of a data item. - In ceph_msg_data_add_pages(), a BUG_ON() was erroneously catching attempts to add page data to a message if the message already had data assigned to it. That was OK until that same commit, at which point it was fine for messages to have multiple data items. It slipped through because that BUG_ON() call was present twice in that function. (You can never be too careful.) In addition two other minor things are changed: - In ceph_msg_data_cursor_init(), the local variable "data" was getting assigned twice. - In ceph_msg_data_advance(), it was assumed that the type-specific advance routine would set new_piece to true after it advanced past the last piece. That may have been fine, but since we check for that case we might as well set it explicitly in ceph_msg_data_advance(). This resolves: http://tracker.ceph.com/issues/4762 Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
-rw-r--r--net/ceph/messenger.c8
1 files changed, 3 insertions, 5 deletions
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index a36d98d8073e..91dd45113c7b 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -913,7 +913,7 @@ ceph_msg_data_pagelist_cursor_init(struct ceph_msg_data_cursor *cursor,
913 cursor->resid = min(length, pagelist->length); 913 cursor->resid = min(length, pagelist->length);
914 cursor->page = page; 914 cursor->page = page;
915 cursor->offset = 0; 915 cursor->offset = 0;
916 cursor->last_piece = length <= PAGE_SIZE; 916 cursor->last_piece = cursor->resid <= PAGE_SIZE;
917} 917}
918 918
919static struct page * 919static struct page *
@@ -1013,8 +1013,6 @@ static void ceph_msg_data_cursor_init(struct ceph_msg *msg, size_t length)
1013 BUG_ON(length > msg->data_length); 1013 BUG_ON(length > msg->data_length);
1014 BUG_ON(list_empty(&msg->data)); 1014 BUG_ON(list_empty(&msg->data));
1015 1015
1016 data = list_first_entry(&msg->data, struct ceph_msg_data, links);
1017
1018 cursor->data_head = &msg->data; 1016 cursor->data_head = &msg->data;
1019 cursor->total_resid = length; 1017 cursor->total_resid = length;
1020 data = list_first_entry(&msg->data, struct ceph_msg_data, links); 1018 data = list_first_entry(&msg->data, struct ceph_msg_data, links);
@@ -1088,14 +1086,15 @@ static bool ceph_msg_data_advance(struct ceph_msg_data_cursor *cursor,
1088 break; 1086 break;
1089 } 1087 }
1090 cursor->total_resid -= bytes; 1088 cursor->total_resid -= bytes;
1091 cursor->need_crc = new_piece;
1092 1089
1093 if (!cursor->resid && cursor->total_resid) { 1090 if (!cursor->resid && cursor->total_resid) {
1094 WARN_ON(!cursor->last_piece); 1091 WARN_ON(!cursor->last_piece);
1095 BUG_ON(list_is_last(&cursor->data->links, cursor->data_head)); 1092 BUG_ON(list_is_last(&cursor->data->links, cursor->data_head));
1096 cursor->data = list_entry_next(cursor->data, links); 1093 cursor->data = list_entry_next(cursor->data, links);
1097 __ceph_msg_data_cursor_init(cursor); 1094 __ceph_msg_data_cursor_init(cursor);
1095 new_piece = true;
1098 } 1096 }
1097 cursor->need_crc = new_piece;
1099 1098
1100 return new_piece; 1099 return new_piece;
1101} 1100}
@@ -3019,7 +3018,6 @@ void ceph_msg_data_add_pages(struct ceph_msg *msg, struct page **pages,
3019 data->length = length; 3018 data->length = length;
3020 data->alignment = alignment & ~PAGE_MASK; 3019 data->alignment = alignment & ~PAGE_MASK;
3021 3020
3022 BUG_ON(!list_empty(&msg->data));
3023 list_add_tail(&data->links, &msg->data); 3021 list_add_tail(&data->links, &msg->data);
3024 msg->data_length += length; 3022 msg->data_length += length;
3025} 3023}