All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <elder@inktank.com>
To: ceph-devel <ceph-devel@vger.kernel.org>
Subject: [PATCH] libceph: fix two messenger bugs
Date: Fri, 19 Apr 2013 17:49:24 -0500	[thread overview]
Message-ID: <5171C9F4.1070108@inktank.com> (raw)
In-Reply-To: <5171C963.2050402@inktank.com>

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>
---
 net/ceph/messenger.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index a36d98d..91dd451 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,
 	cursor->resid = min(length, pagelist->length);
 	cursor->page = page;
 	cursor->offset = 0;
-	cursor->last_piece = length <= PAGE_SIZE;
+	cursor->last_piece = cursor->resid <= PAGE_SIZE;
 }

 static struct page *
@@ -1013,8 +1013,6 @@ static void ceph_msg_data_cursor_init(struct
ceph_msg *msg, size_t length)
 	BUG_ON(length > msg->data_length);
 	BUG_ON(list_empty(&msg->data));

-	data = list_first_entry(&msg->data, struct ceph_msg_data, links);
-
 	cursor->data_head = &msg->data;
 	cursor->total_resid = length;
 	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,
 		break;
 	}
 	cursor->total_resid -= bytes;
-	cursor->need_crc = new_piece;

 	if (!cursor->resid && cursor->total_resid) {
 		WARN_ON(!cursor->last_piece);
 		BUG_ON(list_is_last(&cursor->data->links, cursor->data_head));
 		cursor->data = list_entry_next(cursor->data, links);
 		__ceph_msg_data_cursor_init(cursor);
+		new_piece = true;
 	}
+	cursor->need_crc = new_piece;

 	return new_piece;
 }
@@ -3019,7 +3018,6 @@ void ceph_msg_data_add_pages(struct ceph_msg *msg,
struct page **pages,
 	data->length = length;
 	data->alignment = alignment & ~PAGE_MASK;

-	BUG_ON(!list_empty(&msg->data));
 	list_add_tail(&data->links, &msg->data);
 	msg->data_length += length;
 }
-- 
1.7.9.5


  reply	other threads:[~2013-04-19 22:49 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-19 22:46 [PATCH 0] rbd: layered writes Alex Elder
2013-04-19 22:49 ` Alex Elder [this message]
2013-04-22  7:14   ` [PATCH] libceph: fix two messenger bugs Josh Durgin
2013-04-19 22:49 ` [PATCH] libceph: support pages for class request data Alex Elder
2013-04-22  7:15   ` Josh Durgin
2013-04-19 22:49 ` [PATCH 1/4] rbd: define separate read and write format funcs Alex Elder
2013-04-22  7:23   ` Josh Durgin
2013-04-19 22:50 ` [PATCH 2/4] rbd: encapsulate submission of image object requests Alex Elder
2013-04-22  7:35   ` Josh Durgin
2013-04-19 22:50 ` [PATCH 3/4] rbd: define zero_pages() Alex Elder
2013-04-22  8:05   ` Josh Durgin
2013-04-22 12:35     ` Alex Elder
2013-04-19 22:50 ` [PATCH 4/4] rbd: support page array image requests Alex Elder
2013-04-22  8:13   ` Josh Durgin
2013-04-19 22:50 ` [PATCH 1/2] rbd: implement full object parent reads Alex Elder
2013-04-22 18:13   ` Josh Durgin
2013-04-19 22:50 ` [PATCH 2/2] rbd: issue a copyup for layered writes Alex Elder
2013-04-22 18:16   ` Josh Durgin
2013-04-19 22:52 ` [PATCH 0] rbd: " Alex Elder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5171C9F4.1070108@inktank.com \
    --to=elder@inktank.com \
    --cc=ceph-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.