All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <elder@inktank.com>
To: "ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>
Subject: [PATCH 2/2] rbd: don't take extra bio reference for osd client
Date: Wed, 30 Jan 2013 14:49:12 -0600	[thread overview]
Message-ID: <51098748.7080601@inktank.com> (raw)
In-Reply-To: <510986E4.7050901@inktank.com>

Currently, if the OSD client finds an osd request has had a bio list
attached to it, it drops a reference to it (or rather, to the first
entry on that list) when the request is released.

The code that added that reference (i.e., the rbd client) is
therefore required to take an extra reference to that first bio
structure.

The osd client doesn't really do anything with the bio pointer other
than transfer it from the osd request structure to outgoing (for
writes) and ingoing (for reads) messages.  So it really isn't the
right place to be taking or dropping references.

Furthermore, the rbd client already holds references to all bio
structures it passes to the osd client, and holds them until the
request is completed.  So there's no need for this extra reference
whatsoever.

So remove the bio_put() call in ceph_osdc_release_request(), as
well as its matching bio_get() call in rbd_osd_req_create().

This change could lead to a crash if old libceph.ko was used with
new rbd.ko.  Add a compatibility check at rbd initialization time to
avoid this possibilty.

This resolves:
    http://tracker.ceph.com/issues/3798    and
    http://tracker.ceph.com/issues/3799

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c    |    3 ++-
 net/ceph/ceph_common.c |    2 +-
 net/ceph/osd_client.c  |    4 ----
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index a9ce58c..6586800 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1342,7 +1342,6 @@ static struct ceph_osd_request *rbd_osd_req_create(
 	case OBJ_REQUEST_BIO:
 		rbd_assert(obj_request->bio_list != NULL);
 		osd_req->r_bio = obj_request->bio_list;
-		bio_get(osd_req->r_bio);
 		/* osd client requires "num pages" even for bio */
 		osd_req->r_num_pages = calc_pages_for(offset, length);
 		break;
@@ -4150,6 +4149,8 @@ int __init rbd_init(void)
 {
 	int rc;

+	if (!libceph_compatible(NULL))
+		return -EINVAL;
 	rc = rbd_sysfs_init();
 	if (rc)
 		return rc;
diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c
index a98c03f..c236c235 100644
--- a/net/ceph/ceph_common.c
+++ b/net/ceph/ceph_common.c
@@ -39,7 +39,7 @@
  */
 bool libceph_compatible(void *data)
 {
-	return false;
+	return true;
 }
 EXPORT_SYMBOL(libceph_compatible);

diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index 500ae8b..ba03648 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c
@@ -147,10 +147,6 @@ void ceph_osdc_release_request(struct kref *kref)
 	if (req->r_own_pages)
 		ceph_release_page_vector(req->r_pages,
 					 req->r_num_pages);
-#ifdef CONFIG_BLOCK
-	if (req->r_bio)
-		bio_put(req->r_bio);
-#endif
 	ceph_put_snap_context(req->r_snapc);
 	ceph_pagelist_release(&req->r_trail);
 	if (req->r_mempool)
-- 
1.7.9.5


  parent reply	other threads:[~2013-01-30 20:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-30 20:47 [PATCH 0/2] libceph: osd_client should not take bio reference Alex Elder
2013-01-30 20:49 ` [PATCH 1/2] libceph: add a compatibility check interface Alex Elder
2013-01-31  0:29   ` Josh Durgin
2013-01-30 20:49 ` Alex Elder [this message]
2013-01-31  0:33   ` [PATCH 2/2] rbd: don't take extra bio reference for osd client Josh Durgin
2013-01-31 12:33     ` 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=51098748.7080601@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.