From patchwork Thu Jul 19 12:35:00 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Elder X-Patchwork-Id: 1216431 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id C1752E006E for ; Thu, 19 Jul 2012 12:35:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751434Ab2GSMfD (ORCPT ); Thu, 19 Jul 2012 08:35:03 -0400 Received: from mail-gh0-f174.google.com ([209.85.160.174]:39184 "EHLO mail-gh0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751011Ab2GSMfC (ORCPT ); Thu, 19 Jul 2012 08:35:02 -0400 Received: by mail-gh0-f174.google.com with SMTP id r11so2682359ghr.19 for ; Thu, 19 Jul 2012 05:35:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:content-type:content-transfer-encoding :x-gm-message-state; bh=FjTpGcEshp/FtkN0qP9yCBAtuJgHhD3rULRI0MVa+Zo=; b=IG18VBAVAKB0sqLfQNY/779WLJoHvpF8jqFiwOnWd38kAoJplNPFMpQfXpipUFAUn/ 9dv3SvOHMq+qc4rw+Pn4oUOn0Tm+yepRzzApjZ56CXO54NkXR/f0uxY9oXZauSbve2Mb eQ/qNHOqXR62mXKL33nnvpAp8mbo2U5K0ydUEtQyL4v9blD9CDLUSjXwilhXG1v3RgdY 3r721ygP6u8zJgC6sBQX3Hmp0DKVVCcZ4M7LcAvdIKzURZkP3cp5oiTo10X81IZBrLcT kFKDEtIzMDDg7ahAYn1WjYO6qe5u/FgzLi55U3vWLYxIacMW0T0HmOVSgVsTH5K/PDET kgNg== Received: by 10.50.180.138 with SMTP id do10mr1220689igc.36.1342701301452; Thu, 19 Jul 2012 05:35:01 -0700 (PDT) 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 wo5sm7132186igc.0.2012.07.19.05.35.00 (version=SSLv3 cipher=OTHER); Thu, 19 Jul 2012 05:35:01 -0700 (PDT) Message-ID: <5007FEF4.5060401@inktank.com> Date: Thu, 19 Jul 2012 07:35:00 -0500 From: Alex Elder User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 MIME-Version: 1.0 To: "ceph-devel@vger.kernel.org" Subject: [PATCH 5/6] rbd: use reference counting for the snap context References: <5007FDEA.9010605@inktank.com> In-Reply-To: <5007FDEA.9010605@inktank.com> X-Gm-Message-State: ALoCoQkHHQJWFBzn/6tZAp4oYSiX+rQMshOPwqKYN+zgeK5+hMR+EjlKJ5l6YA2EJr/NZ2Z75dYx Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org This prevents a race between requests with a given snap context and header updates that free it. The osd client was already expecting the snap context to be reference counted, since it get()s it in ceph_osdc_build_request and put()s it when the request completes. Also remove the second down_read()/up_read() on header_rwsem in rbd_do_request, which wasn't actually preventing this race or protecting any other data. Signed-off-by: Josh Durgin Reviewed-by: Alex Elder --- drivers/block/rbd.c | 36 ++++++++++++++++++------------------ 1 files changed, 18 insertions(+), 18 deletions(-) /* @@ -902,13 +902,10 @@ static int rbd_do_request(struct request *rq, dout("rbd_do_request object_name=%s ofs=%lld len=%lld\n", object_name, len, ofs); - down_read(&rbd_dev->header_rwsem); - osdc = &rbd_dev->rbd_client->client->osdc; req = ceph_osdc_alloc_request(osdc, flags, snapc, ops, false, GFP_NOIO, pages, bio); if (!req) { - up_read(&rbd_dev->header_rwsem); ret = -ENOMEM; goto done_pages; } @@ -942,7 +939,6 @@ static int rbd_do_request(struct request *rq, snapc, &mtime, req->r_oid, req->r_oid_len); - up_read(&rbd_dev->header_rwsem); if (linger_req) { ceph_osdc_set_request_linger(osdc, req); @@ -1448,6 +1444,7 @@ static void rbd_rq_fn(struct request_queue *q) u64 ofs; int num_segs, cur_seg = 0; struct rbd_req_coll *coll; + struct ceph_snap_context *snapc; /* peek at request from block layer */ if (!rq) @@ -1474,21 +1471,20 @@ static void rbd_rq_fn(struct request_queue *q) spin_unlock_irq(q->queue_lock); - if (rbd_dev->snap_id != CEPH_NOSNAP) { - bool snap_exists; + down_read(&rbd_dev->header_rwsem); - down_read(&rbd_dev->header_rwsem); - snap_exists = rbd_dev->snap_exists; + if (rbd_dev->snap_id != CEPH_NOSNAP && !rbd_dev->snap_exists) { up_read(&rbd_dev->header_rwsem); - - if (!snap_exists) { - dout("request for non-existent snapshot"); - spin_lock_irq(q->queue_lock); - __blk_end_request_all(rq, -ENXIO); - continue; - } + dout("request for non-existent snapshot"); + spin_lock_irq(q->queue_lock); + __blk_end_request_all(rq, -ENXIO); + continue; } + snapc = ceph_get_snap_context(rbd_dev->header.snapc); + + up_read(&rbd_dev->header_rwsem); + dout("%s 0x%x bytes at 0x%llx\n", do_write ? "write" : "read", size, blk_rq_pos(rq) * SECTOR_SIZE); @@ -1498,6 +1494,7 @@ static void rbd_rq_fn(struct request_queue *q) if (!coll) { spin_lock_irq(q->queue_lock); __blk_end_request_all(rq, -ENOMEM); + ceph_put_snap_context(snapc); continue; } @@ -1521,7 +1518,7 @@ static void rbd_rq_fn(struct request_queue *q) /* init OSD command: write or read */ if (do_write) rbd_req_write(rq, rbd_dev, - rbd_dev->header.snapc, + snapc, ofs, op_size, bio, coll, cur_seg); @@ -1544,6 +1541,8 @@ next_seg: if (bp) bio_pair_release(bp); spin_lock_irq(q->queue_lock); + + ceph_put_snap_context(snapc); } } @@ -1744,7 +1743,8 @@ static int __rbd_refresh_header(struct rbd_device *rbd_dev) /* rbd_dev->header.object_prefix shouldn't change */ kfree(rbd_dev->header.snap_sizes); kfree(rbd_dev->header.snap_names); - kfree(rbd_dev->header.snapc); + /* osd requests may still refer to snapc */ + ceph_put_snap_context(rbd_dev->header.snapc); rbd_dev->header.image_size = h.image_size; rbd_dev->header.total_snaps = h.total_snaps; diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index a6bbda2..988f944 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -626,7 +626,7 @@ static void rbd_header_free(struct rbd_image_header *header) kfree(header->object_prefix); kfree(header->snap_sizes); kfree(header->snap_names); - kfree(header->snapc); + ceph_put_snap_context(header->snapc); }