From patchwork Fri Feb 22 17:17:20 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Elder X-Patchwork-Id: 2176551 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 754D63FE37 for ; Fri, 22 Feb 2013 17:17:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758336Ab3BVRR0 (ORCPT ); Fri, 22 Feb 2013 12:17:26 -0500 Received: from mail-ia0-f179.google.com ([209.85.210.179]:64258 "EHLO mail-ia0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758313Ab3BVRRX (ORCPT ); Fri, 22 Feb 2013 12:17:23 -0500 Received: by mail-ia0-f179.google.com with SMTP id x24so737631iak.10 for ; Fri, 22 Feb 2013 09:17:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding :x-gm-message-state; bh=/rUzb9ejXMmzqfwC/W7v9mtjLBu6KFgJgyyHjaZ9wNw=; b=iYvMSHpeOipB8+cTr4Mip6z6kinRsPu8MOmfYTgTUFYiM5u/0zVs+KARttncRFQOe6 vH5ixUjafv1aPBhXxVXuqOU+EiE/86tGnCnBUS/HnKFYo42ogyjetWki3lGzNX7X1HP9 z2mX63h/j0xwB/tc3wIl6NdVWleNzELpQ+BLA03o0qwtwtP2aSIWI2vTeMatZKNj+iAz 1Wo3f+dAFjDvV446GLKeQEV6PbH9LfxzMsiC81s1j+GS0wExKZdf3EtiSvFCGaz3a/hY 14H3iX/KHjJ4+ynpRFg9sHYsMwv+DCBDhz/7GgTDLQeO9wOd/Njs3b4vpCijkFFX4DbS 42OQ== X-Received: by 10.50.135.105 with SMTP id pr9mr12040igb.6.1361553442682; Fri, 22 Feb 2013 09:17:22 -0800 (PST) Received: from [172.22.22.4] (c-71-195-31-37.hsd1.mn.comcast.net. [71.195.31.37]) by mx.google.com with ESMTPS id ww6sm2215550igb.2.2013.02.22.09.17.20 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 22 Feb 2013 09:17:21 -0800 (PST) Message-ID: <5127A820.6070505@inktank.com> Date: Fri, 22 Feb 2013 11:17:20 -0600 From: Alex Elder User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-Version: 1.0 To: ceph-devel Subject: [PATCH] rbd: barriers are hard References: <5127A7C1.3070609@inktank.com> In-Reply-To: <5127A7C1.3070609@inktank.com> X-Gm-Message-State: ALoCoQkfZbe+FTpVT3G0O0ZxpPnWK3AGoScJw9Hol1fpb8FgTUGI8NA7rQw9vtcuhwqLEezKYS6w Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org 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 --- 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; }