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] rbd: barriers are hard
Date: Fri, 22 Feb 2013 11:17:20 -0600	[thread overview]
Message-ID: <5127A820.6070505@inktank.com> (raw)
In-Reply-To: <5127A7C1.3070609@inktank.com>

Let's go shopping!

I'm afraid this may not have gotten it right:
    07741308  rbd: add barriers near done flag operations

The smp_wmb() should have been done *before* setting the done flag,
to ensure all other data was valid before marking the object request
done.

Switch to use atomic_inc_return() here to set the done flag, which
allows us to verify we don't mark something done more than once.
Doing this also implies general barriers before and after the call.

And although a read memory barrier might have been sufficient before
reading the done flag, convert this to a full memory barrier just
to put this issue to bed.

This resolves:
    http://tracker.ceph.com/issues/4238

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 3cc003b..bd6078b 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1226,13 +1226,22 @@ static void obj_request_done_init(struct
rbd_obj_request *obj_request)

 static void obj_request_done_set(struct rbd_obj_request *obj_request)
 {
-	atomic_set(&obj_request->done, 1);
-	smp_wmb();
+	int done;
+
+	done = atomic_inc_return(&obj_request->done);
+	if (done > 1) {
+		struct rbd_img_request *img_request = obj_request->img_request;
+		struct rbd_device *rbd_dev;
+
+		rbd_dev = img_request ? img_request->rbd_dev : NULL;
+		rbd_warn(rbd_dev, "obj_request %p was already done\n",
+			obj_request);
+	}
 }

 static bool obj_request_done_test(struct rbd_obj_request *obj_request)
 {
-	smp_rmb();
+	smp_mb();
 	return atomic_read(&obj_request->done) != 0;
 }

-- 
1.7.9.5


  parent reply	other threads:[~2013-02-22 17:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-22 17:15 Four miscellaneous patches Alex Elder
2013-02-22 17:17 ` [PATCH] rbd: ignore zero-length requests Alex Elder
2013-02-22 17:17 ` Alex Elder [this message]
2013-02-22 17:17 ` [PATCH] rbd: normalize dout() calls Alex Elder
2013-02-22 17:17 ` [PATCH] libceph: define connection flag helpers Alex Elder
2013-02-25 19:11 ` Four miscellaneous patches Josh Durgin

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=5127A820.6070505@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.