From patchwork Thu Aug 28 13:38:01 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sage Weil X-Patchwork-Id: 36204 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-oi0-f69.google.com (mail-oi0-f69.google.com [209.85.218.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 034472054F for ; Thu, 28 Aug 2014 13:39:57 +0000 (UTC) Received: by mail-oi0-f69.google.com with SMTP id i138sf7199681oig.4 for ; Thu, 28 Aug 2014 06:39:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:cc:subject :date:message-id:in-reply-to:references:sender:precedence:list-id :x-original-sender:x-original-authentication-results:mailing-list :list-post:list-help:list-archive:list-unsubscribe; bh=HOBzBiuaKLIvFn890XD7Z3cRyoCSJxUVVcmcwKl9vt8=; b=OMZfmcE+D7YZyaGQy89tDVl56QRVVT8eKz3/X13NbkhMeAMuVeN6OSGieuYgE/TPZ6 0qd4SdTdUVY/MTKzfqJKHzHdR0sVYJCQDFUq+CU/LxavsKxpY1+W7KAYBEs0VPnLdXx4 AIvuAz2o16Lecxl4G+mxn+XH5fiMPQg5DCpGJX3+FF0KxZf8joSg3zcWDIPKNMae7/Mu nPQsCCR2ofzqC1TYbDqvsoOWmwCFn0SBZMnOh0zIEej6ZbzkuZwULC8he0NRm+ATeqny SRdgY66tiOro/F3kQiJpYp1UT/hgeRHyo37gAPqwEuGvttuXxp7XnqoHlaBjHt64wbMt +rag== X-Gm-Message-State: ALoCoQmwdbYoKoC6T0KuDWSuPFC8rkzF1R9P5Mvi1XIbF6VM1Gg4KGhBkp85v67IpWyl77Jyj5LQ X-Received: by 10.182.107.135 with SMTP id hc7mr2021695obb.48.1409233197615; Thu, 28 Aug 2014 06:39:57 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.23.201 with SMTP id 67ls573917qgp.45.gmail; Thu, 28 Aug 2014 06:39:57 -0700 (PDT) X-Received: by 10.52.38.67 with SMTP id e3mr694165vdk.56.1409233197480; Thu, 28 Aug 2014 06:39:57 -0700 (PDT) Received: from mail-vc0-f176.google.com (mail-vc0-f176.google.com [209.85.220.176]) by mx.google.com with ESMTPS id i9si3442283vcl.42.2014.08.28.06.39.57 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 28 Aug 2014 06:39:57 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.176 as permitted sender) client-ip=209.85.220.176; Received: by mail-vc0-f176.google.com with SMTP id ik5so790756vcb.7 for ; Thu, 28 Aug 2014 06:39:57 -0700 (PDT) X-Received: by 10.52.119.229 with SMTP id kx5mr717872vdb.40.1409233197393; Thu, 28 Aug 2014 06:39:57 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.45.67 with SMTP id uj3csp238297vcb; Thu, 28 Aug 2014 06:39:56 -0700 (PDT) X-Received: by 10.68.171.33 with SMTP id ar1mr6076138pbc.148.1409233196477; Thu, 28 Aug 2014 06:39:56 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id lr3si6523982pab.140.2014.08.28.06.39.52 for ; Thu, 28 Aug 2014 06:39:53 -0700 (PDT) Received-SPF: none (google.com: stable-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751277AbaH1Njt (ORCPT + 1 other); Thu, 28 Aug 2014 09:39:49 -0400 Received: from cobra.newdream.net ([66.33.216.30]:58975 "EHLO cobra.newdream.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751261AbaH1Njt (ORCPT ); Thu, 28 Aug 2014 09:39:49 -0400 Received: from flab.front.sepia.ceph.com (gw.sepia.ceph.com [64.90.32.62]) by cobra.newdream.net (Postfix) with ESMTPA id F388F814B0; Thu, 28 Aug 2014 06:39:48 -0700 (PDT) From: sweil@redhat.com To: rhkernel-list@redhat.com Cc: Alex Elder , stable@vger.kernel.org Subject: [PATCH 256/303] rbd: use reference counts for image requests Date: Thu, 28 Aug 2014 06:38:01 -0700 Message-Id: <1409233128-9919-257-git-send-email-sweil@redhat.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1409233128-9919-1-git-send-email-sweil@redhat.com> References: <1409233128-9919-1-git-send-email-sweil@redhat.com> Sender: stable-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: stable@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: sweil@redhat.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.176 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , From: Alex Elder Each image request contains a reference count, but to date it has not actually been used. (I think this was just an oversight.) A recent report involving rbd failing an assertion shed light on why and where we need to use these reference counts. Every OSD request associated with an object request uses rbd_osd_req_callback() as its callback function. That function will call a helper function (dependent on the type of OSD request) that will set the object request's "done" flag if the object request if appropriate. If that "done" flag is set, the object request is passed to rbd_obj_request_complete(). In rbd_obj_request_complete(), requests are processed in sequential order. So if an object request completes before one of its predecessors in the image request, the completion is deferred. Otherwise, if it's a completing object's "turn" to be completed, it is passed to rbd_img_obj_end_request(), which records the result of the operation, accumulates transferred bytes, and so on. Next, the successor to this request is checked and if it is marked "done", (deferred) completion processing is performed on that request, and so on. If the last object request in an image request is completed, rbd_img_request_complete() is called, which (typically) destroys the image request. There is a race here, however. The instant an object request is marked "done" it can be provided (by a thread handling completion of one of its predecessor operations) to rbd_img_obj_end_request(), which (for the last request) can then lead to the image request getting torn down. And this can happen *before* that object has itself entered rbd_img_obj_end_request(). As a result, once it *does* enter that function, the image request (and even the object request itself) may have been freed and become invalid. All that's necessary to avoid this is to properly count references to the image requests. We tear down an image request's object requests all at once--only when the entire image request has completed. So there's no need for an image request to count references for its object requests. However, we don't want an image request to go away until the last of its object requests has passed through rbd_img_obj_callback(). In other words, we don't want rbd_img_request_complete() to necessarily result in the image request being destroyed, because it may get called before we've finished processing on all of its object requests. So the fix is to add a reference to an image request for each of its object requests. The reference can be viewed as representing an object request that has not yet finished its call to rbd_img_obj_callback(). That is emphasized by getting the reference right after assigning that as the image object's callback function. The corresponding release of that reference is done at the end of rbd_img_obj_callback(), which every image object request passes through exactly once. Cc: stable@vger.kernel.org Signed-off-by: Alex Elder Reviewed-by: Ilya Dryomov (cherry picked from commit 0f2d5be792b0466b06797f637cfbb0f64dbb408c) --- drivers/block/rbd.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 7857279..5dee05d 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -1426,6 +1426,13 @@ static void rbd_obj_request_put(struct rbd_obj_request *obj_request) kref_put(&obj_request->kref, rbd_obj_request_destroy); } +static void rbd_img_request_get(struct rbd_img_request *img_request) +{ + dout("%s: img %p (was %d)\n", __func__, img_request, + atomic_read(&img_request->kref.refcount)); + kref_get(&img_request->kref); +} + static bool img_request_child_test(struct rbd_img_request *img_request); static void rbd_parent_request_destroy(struct kref *kref); static void rbd_img_request_destroy(struct kref *kref); @@ -2186,6 +2193,7 @@ static void rbd_img_obj_callback(struct rbd_obj_request *obj_request) img_request->next_completion = which; out: spin_unlock_irq(&img_request->completion_lock); + rbd_img_request_put(img_request); if (!more) rbd_img_request_complete(img_request); @@ -2285,6 +2293,7 @@ static int rbd_img_request_fill(struct rbd_img_request *img_request, goto out_unwind; obj_request->osd_req = osd_req; obj_request->callback = rbd_img_obj_callback; + rbd_img_request_get(img_request); if (write_request) { osd_req_op_alloc_hint_init(osd_req, which,